From 715bff3ebfa4125f0d7bcd43eec11da3c1577403 Mon Sep 17 00:00:00 2001 From: hartytp Date: Tue, 25 Jan 2022 02:02:15 +0000 Subject: [PATCH 1/8] Revert "Merge pull request #1544 from airwoodix/dataset-compression" (#1838) * Revert "Merge pull request #1544 from airwoodix/dataset-compression" This reverts commit 311a818a490eca8df91defae0e76ff9426e4b741, reversing changes made to 7ffe4dc2e3ff9a83cf861be7c4b0ec78e4d4c772. * fix accidental revert of f42bea06a89ef80de4dcd4d996544a97287c969f --- RELEASE_NOTES.rst | 8 ------ artiq/applets/big_number.py | 2 +- artiq/applets/image.py | 2 +- artiq/applets/plot_hist.py | 4 +-- artiq/applets/plot_xy.py | 9 +++---- artiq/applets/plot_xy_hist.py | 6 ++--- artiq/applets/simple.py | 3 +-- artiq/browser/datasets.py | 4 +-- artiq/dashboard/datasets.py | 18 ++++++------- artiq/language/environment.py | 13 ++------- artiq/master/databases.py | 39 +++++---------------------- artiq/master/worker_db.py | 38 +++++++++----------------- artiq/test/test_datasets.py | 50 +++-------------------------------- artiq/test/test_scheduler.py | 10 ++----- 14 files changed, 50 insertions(+), 156 deletions(-) diff --git a/RELEASE_NOTES.rst b/RELEASE_NOTES.rst index a97f43d5e..fdb45ed7a 100644 --- a/RELEASE_NOTES.rst +++ b/RELEASE_NOTES.rst @@ -32,9 +32,6 @@ Highlights: * Previously detected RTIO async errors are reported to the host after each kernel terminates and a warning is logged. The warning is additional to the one already printed in the core device log upon detection of the error. -* HDF5 options can now be passed when creating datasets with ``set_dataset``. This allows - in particular to use transparent compression filters as follows: - ``set_dataset(name, value, hdf5_options={"compression": "gzip"})``. * Removed worker DB warning for writing a dataset that is also in the archive Breaking changes: @@ -51,10 +48,6 @@ Breaking changes: calling `ADF5356.init()`. * DRTIO: Changed message alignment from 32-bits to 64-bits. * The deprecated ``set_dataset(..., save=...)`` is no longer supported. -* The internal dataset representation was changed to support tracking HDF5 options like e.g. - a compression method. This requires changes to code reading the dataset persistence file - (``dataset_db.pyon``) and to custom applets. - ARTIQ-6 ------- @@ -129,7 +122,6 @@ Breaking changes: * Experiment classes with underscore-prefixed names are now ignored when ``artiq_client`` determines which experiment to submit (consistent with ``artiq_run``). - ARTIQ-5 ------- diff --git a/artiq/applets/big_number.py b/artiq/applets/big_number.py index a0714b734..62348c8cf 100755 --- a/artiq/applets/big_number.py +++ b/artiq/applets/big_number.py @@ -13,7 +13,7 @@ class NumberWidget(QtWidgets.QLCDNumber): def data_changed(self, data, mods): try: - n = float(data[self.dataset_name]["value"]) + n = float(data[self.dataset_name][1]) except (KeyError, ValueError, TypeError): n = "---" self.display(n) diff --git a/artiq/applets/image.py b/artiq/applets/image.py index 4bc6b5f86..b7d36c1a1 100755 --- a/artiq/applets/image.py +++ b/artiq/applets/image.py @@ -13,7 +13,7 @@ class Image(pyqtgraph.ImageView): def data_changed(self, data, mods): try: - img = data[self.args.img]["value"] + img = data[self.args.img][1] except KeyError: return self.setImage(img) diff --git a/artiq/applets/plot_hist.py b/artiq/applets/plot_hist.py index 298246a5c..b3493c836 100755 --- a/artiq/applets/plot_hist.py +++ b/artiq/applets/plot_hist.py @@ -17,11 +17,11 @@ class HistogramPlot(pyqtgraph.PlotWidget): def data_changed(self, data, mods, title): try: - y = data[self.args.y]["value"] + y = data[self.args.y][1] if self.args.x is None: x = None else: - x = data[self.args.x]["value"] + x = data[self.args.x][1] except KeyError: return if x is None: diff --git a/artiq/applets/plot_xy.py b/artiq/applets/plot_xy.py index 32bf95b50..8eaf9786a 100755 --- a/artiq/applets/plot_xy.py +++ b/artiq/applets/plot_xy.py @@ -6,7 +6,6 @@ from PyQt5.QtCore import QTimer import pyqtgraph from artiq.applets.simple import TitleApplet -from artiq.master.databases import make_dataset as empty_dataset class XYPlot(pyqtgraph.PlotWidget): @@ -22,14 +21,14 @@ class XYPlot(pyqtgraph.PlotWidget): def data_changed(self, data, mods, title): try: - y = data[self.args.y]["value"] + y = data[self.args.y][1] except KeyError: return - x = data.get(self.args.x, empty_dataset())["value"] + x = data.get(self.args.x, (False, None))[1] if x is None: x = np.arange(len(y)) - error = data.get(self.args.error, empty_dataset())["value"] - fit = data.get(self.args.fit, empty_dataset())["value"] + error = data.get(self.args.error, (False, None))[1] + fit = data.get(self.args.fit, (False, None))[1] if not len(y) or len(y) != len(x): self.mismatch['X values'] = True diff --git a/artiq/applets/plot_xy_hist.py b/artiq/applets/plot_xy_hist.py index 01eab5421..39bd49098 100755 --- a/artiq/applets/plot_xy_hist.py +++ b/artiq/applets/plot_xy_hist.py @@ -126,9 +126,9 @@ class XYHistPlot(QtWidgets.QSplitter): def data_changed(self, data, mods): try: - xs = data[self.args.xs]["value"] - histogram_bins = data[self.args.histogram_bins]["value"] - histograms_counts = data[self.args.histograms_counts]["value"] + xs = data[self.args.xs][1] + histogram_bins = data[self.args.histogram_bins][1] + histograms_counts = data[self.args.histograms_counts][1] except KeyError: return if len(xs) != histograms_counts.shape[0]: diff --git a/artiq/applets/simple.py b/artiq/applets/simple.py index db6f2334d..e5776310a 100644 --- a/artiq/applets/simple.py +++ b/artiq/applets/simple.py @@ -10,7 +10,6 @@ from sipyco.sync_struct import Subscriber, process_mod from sipyco import pyon from sipyco.pipe_ipc import AsyncioChildComm -from artiq.master.databases import make_dataset as empty_dataset logger = logging.getLogger(__name__) @@ -252,7 +251,7 @@ class TitleApplet(SimpleApplet): def emit_data_changed(self, data, mod_buffer): if self.args.title is not None: - title_values = {k.replace(".", "/"): data.get(k, empty_dataset())["value"] + title_values = {k.replace(".", "/"): data.get(k, (False, None))[1] for k in self.dataset_title} try: title = self.args.title.format(**title_values) diff --git a/artiq/browser/datasets.py b/artiq/browser/datasets.py index d3d8171ac..b66b18216 100644 --- a/artiq/browser/datasets.py +++ b/artiq/browser/datasets.py @@ -104,8 +104,8 @@ class DatasetsDock(QtWidgets.QDockWidget): idx = self.table_model_filter.mapToSource(idx[0]) key = self.table_model.index_to_key(idx) if key is not None: - dataset = self.table_model.backing_store[key] - asyncio.ensure_future(self._upload_dataset(key, dataset["value"])) + persist, value = self.table_model.backing_store[key] + asyncio.ensure_future(self._upload_dataset(key, value)) def save_state(self): return bytes(self.table.header().saveState()) diff --git a/artiq/dashboard/datasets.py b/artiq/dashboard/datasets.py index 23fc995cb..10e51e906 100644 --- a/artiq/dashboard/datasets.py +++ b/artiq/dashboard/datasets.py @@ -146,16 +146,16 @@ class Creator(QtWidgets.QDialog): class Model(DictSyncTreeSepModel): - def __init__(self, init): - DictSyncTreeSepModel.__init__( - self, ".", ["Dataset", "Persistent", "Value"], init - ) + def __init__(self, init): + DictSyncTreeSepModel.__init__(self, ".", + ["Dataset", "Persistent", "Value"], + init) def convert(self, k, v, column): if column == 1: - return "Y" if v["persist"] else "N" + return "Y" if v[0] else "N" elif column == 2: - return short_format(v["value"]) + return short_format(v[1]) else: raise ValueError @@ -223,8 +223,8 @@ class DatasetsDock(QtWidgets.QDockWidget): idx = self.table_model_filter.mapToSource(idx[0]) key = self.table_model.index_to_key(idx) if key is not None: - dataset = self.table_model.backing_store[key] - t = type(dataset["value"]) + persist, value = self.table_model.backing_store[key] + t = type(value) if np.issubdtype(t, np.number): dialog_cls = NumberEditor elif np.issubdtype(t, np.bool_): @@ -235,7 +235,7 @@ class DatasetsDock(QtWidgets.QDockWidget): logger.error("Cannot edit dataset %s: " "type %s is not supported", key, t) return - dialog_cls(self, self.dataset_ctl, key, dataset["value"]).open() + dialog_cls(self, self.dataset_ctl, key, value).open() def delete_clicked(self): idx = self.table.selectedIndexes() diff --git a/artiq/language/environment.py b/artiq/language/environment.py index 819391925..969664d15 100644 --- a/artiq/language/environment.py +++ b/artiq/language/environment.py @@ -330,8 +330,7 @@ class HasEnvironment: @rpc(flags={"async"}) def set_dataset(self, key, value, - broadcast=False, persist=False, archive=True, - hdf5_options=None): + broadcast=False, persist=False, archive=True): """Sets the contents and handling modes of a dataset. Datasets must be scalars (``bool``, ``int``, ``float`` or NumPy scalar) @@ -343,16 +342,8 @@ class HasEnvironment: broadcast. :param archive: the data is saved into the local storage of the current run (archived as a HDF5 file). - :param hdf5_options: dict of keyword arguments to pass to - :meth:`h5py.Group.create_dataset`. For example, pass ``{"compression": "gzip"}`` - to enable transparent zlib compression of this dataset in the HDF5 archive. - See the `h5py documentation `_ - for a list of valid options. """ - - self.__dataset_mgr.set( - key, value, broadcast, persist, archive, hdf5_options - ) + self.__dataset_mgr.set(key, value, broadcast, persist, archive) @rpc(flags={"async"}) def mutate_dataset(self, key, index, value): diff --git a/artiq/master/databases.py b/artiq/master/databases.py index 8ef71c6a2..14cfae4cd 100644 --- a/artiq/master/databases.py +++ b/artiq/master/databases.py @@ -35,15 +35,6 @@ class DeviceDB: return desc -def make_dataset(*, persist=False, value=None, hdf5_options=None): - "PYON-serializable representation of a dataset in the DatasetDB" - return { - "persist": persist, - "value": value, - "hdf5_options": hdf5_options or {}, - } - - class DatasetDB(TaskObject): def __init__(self, persist_file, autosave_period=30): self.persist_file = persist_file @@ -53,23 +44,10 @@ class DatasetDB(TaskObject): file_data = pyon.load_file(self.persist_file) except FileNotFoundError: file_data = dict() - self.data = Notifier( - { - k: make_dataset( - persist=True, - value=v["value"], - hdf5_options=v["hdf5_options"] - ) - for k, v in file_data.items() - } - ) + self.data = Notifier({k: (True, v) for k, v in file_data.items()}) def save(self): - data = { - k: d - for k, d in self.data.raw_view.items() - if d["persist"] - } + data = {k: v[1] for k, v in self.data.raw_view.items() if v[0]} pyon.store_file(self.persist_file, data) async def _do(self): @@ -81,23 +59,20 @@ class DatasetDB(TaskObject): self.save() def get(self, key): - return self.data.raw_view[key] + return self.data.raw_view[key][1] def update(self, mod): process_mod(self.data, mod) # convenience functions (update() can be used instead) - def set(self, key, value, persist=None, hdf5_options=None): + def set(self, key, value, persist=None): if persist is None: if key in self.data.raw_view: - persist = self.data.raw_view[key]["persist"] + persist = self.data.raw_view[key][0] else: persist = False - self.data[key] = make_dataset( - persist=persist, - value=value, - hdf5_options=hdf5_options, - ) + self.data[key] = (persist, value) def delete(self, key): del self.data[key] + # diff --git a/artiq/master/worker_db.py b/artiq/master/worker_db.py index c739731a0..fccdc1c11 100644 --- a/artiq/master/worker_db.py +++ b/artiq/master/worker_db.py @@ -8,12 +8,9 @@ from operator import setitem import importlib import logging -import numpy as np - from sipyco.sync_struct import Notifier from sipyco.pc_rpc import AutoTarget, Client, BestEffortClient -from artiq.master.databases import make_dataset logger = logging.getLogger(__name__) @@ -118,26 +115,17 @@ class DatasetManager: self.ddb = ddb self._broadcaster.publish = ddb.update - def set(self, key, value, broadcast=False, persist=False, archive=True, - hdf5_options=None): + def set(self, key, value, broadcast=False, persist=False, archive=True): if persist: broadcast = True if broadcast: - self._broadcaster[key] = make_dataset( - persist=persist, - value=value, - hdf5_options=hdf5_options, - ) + self._broadcaster[key] = persist, value elif key in self._broadcaster.raw_view: del self._broadcaster[key] if archive: - self.local[key] = make_dataset( - persist=persist, - value=value, - hdf5_options=hdf5_options, - ) + self.local[key] = value elif key in self.local: del self.local[key] @@ -145,11 +133,11 @@ class DatasetManager: target = self.local.get(key, None) if key in self._broadcaster.raw_view: if target is not None: - assert target["value"] is self._broadcaster.raw_view[key]["value"] - return self._broadcaster[key]["value"] + assert target is self._broadcaster.raw_view[key][1] + return self._broadcaster[key][1] if target is None: raise KeyError("Cannot mutate nonexistent dataset '{}'".format(key)) - return target["value"] + return target def mutate(self, key, index, value): target = self._get_mutation_target(key) @@ -165,15 +153,15 @@ class DatasetManager: def get(self, key, archive=False): if key in self.local: - return self.local[key]["value"] - - dataset = self.ddb.get(key) + return self.local[key] + + data = self.ddb.get(key) if archive: if key in self.archive: logger.warning("Dataset '%s' is already in archive, " "overwriting", key, stack_info=True) - self.archive[key] = dataset - return dataset["value"] + self.archive[key] = data + return data def write_hdf5(self, f): datasets_group = f.create_group("datasets") @@ -189,7 +177,7 @@ def _write(group, k, v): # Add context to exception message when the user writes a dataset that is # not representable in HDF5. try: - group.create_dataset(k, data=v["value"], **v["hdf5_options"]) + group[k] = v except TypeError as e: raise TypeError("Error writing dataset '{}' of type '{}': {}".format( - k, type(v["value"]), e)) + k, type(v), e)) diff --git a/artiq/test/test_datasets.py b/artiq/test/test_datasets.py index 3fa6d6bb7..871568a2a 100644 --- a/artiq/test/test_datasets.py +++ b/artiq/test/test_datasets.py @@ -3,9 +3,6 @@ import copy import unittest -import h5py -import numpy as np - from sipyco.sync_struct import process_mod from artiq.experiment import EnvExperiment @@ -17,7 +14,7 @@ class MockDatasetDB: self.data = dict() def get(self, key): - return self.data[key]["value"] + return self.data[key][1] def update(self, mod): # Copy mod before applying to avoid sharing references to objects @@ -85,9 +82,9 @@ class ExperimentDatasetCase(unittest.TestCase): def test_append_broadcast(self): self.exp.set(KEY, [], broadcast=True) self.exp.append(KEY, 0) - self.assertEqual(self.dataset_db.data[KEY]["value"], [0]) + self.assertEqual(self.dataset_db.data[KEY][1], [0]) self.exp.append(KEY, 1) - self.assertEqual(self.dataset_db.data[KEY]["value"], [0, 1]) + self.assertEqual(self.dataset_db.data[KEY][1], [0, 1]) def test_append_array(self): for broadcast in (True, False): @@ -106,44 +103,3 @@ class ExperimentDatasetCase(unittest.TestCase): with self.assertRaises(KeyError): self.exp.append(KEY, 0) - def test_write_hdf5_options(self): - data = np.random.randint(0, 1024, 1024) - self.exp.set( - KEY, - data, - hdf5_options=dict( - compression="gzip", - compression_opts=6, - shuffle=True, - fletcher32=True - ), - ) - - with h5py.File("test.h5", "a", "core", backing_store=False) as f: - self.dataset_mgr.write_hdf5(f) - - self.assertTrue(np.array_equal(f["datasets"][KEY][()], data)) - self.assertEqual(f["datasets"][KEY].compression, "gzip") - self.assertEqual(f["datasets"][KEY].compression_opts, 6) - self.assertTrue(f["datasets"][KEY].shuffle) - self.assertTrue(f["datasets"][KEY].fletcher32) - - def test_write_hdf5_no_options(self): - data = np.random.randint(0, 1024, 1024) - self.exp.set(KEY, data) - - with h5py.File("test.h5", "a", "core", backing_store=False) as f: - self.dataset_mgr.write_hdf5(f) - self.assertTrue(np.array_equal(f["datasets"][KEY][()], data)) - self.assertIsNone(f["datasets"][KEY].compression) - - def test_write_hdf5_invalid_type(self): - class CustomType: - def __init__(self, x): - self.x = x - - self.exp.set(KEY, CustomType(42)) - - with h5py.File("test.h5", "w", "core", backing_store=False) as f: - with self.assertRaisesRegex(TypeError, "CustomType"): - self.dataset_mgr.write_hdf5(f) diff --git a/artiq/test/test_scheduler.py b/artiq/test/test_scheduler.py index 984804fcf..b4327e72e 100644 --- a/artiq/test/test_scheduler.py +++ b/artiq/test/test_scheduler.py @@ -6,7 +6,6 @@ from time import time, sleep from artiq.experiment import * from artiq.master.scheduler import Scheduler -from artiq.master.databases import make_dataset class EmptyExperiment(EnvExperiment): @@ -288,13 +287,8 @@ class SchedulerCase(unittest.TestCase): nonlocal termination_ok self.assertEqual( mod, - { - "action": "setitem", - "key": "termination_ok", - "value": make_dataset(value=True), - "path": [] - } - ) + {"action": "setitem", "key": "termination_ok", + "value": (False, True), "path": []}) termination_ok = True handlers = { "update_dataset": check_termination From 4644e105b169aea58e37b9eaa7ebd252dfc9b3f0 Mon Sep 17 00:00:00 2001 From: pca006132 Date: Sun, 23 Jan 2022 21:10:46 +0800 Subject: [PATCH 2/8] compiler: modified exception representation Exception name is replaced by exception ID, which requires no allocation. Other strings in the exception can now be 'host-only' strings, which is represented by a CSlice with len = usize::MAX and ptr = key, to avoid the need for allocation when raising exceptions through RPC. --- artiq/compiler/builtins.py | 23 +++++++------ artiq/compiler/embedding.py | 24 ++++++++++++++ artiq/compiler/module.py | 3 +- .../compiler/transforms/artiq_ir_generator.py | 32 ++++++++++--------- artiq/coredevice/comm_kernel.py | 26 ++++++++------- 5 files changed, 72 insertions(+), 36 deletions(-) diff --git a/artiq/compiler/builtins.py b/artiq/compiler/builtins.py index 54b25e719..4524d87dd 100644 --- a/artiq/compiler/builtins.py +++ b/artiq/compiler/builtins.py @@ -123,18 +123,23 @@ class TException(types.TMono): # * File, line and column where it was raised (str, int, int). # * Message, which can contain substitutions {0}, {1} and {2} (str). # * Three 64-bit integers, parameterizing the message (numpy.int64). + # These attributes are prefixed with `#` so that users cannot access them, + # and we don't have to do string allocation in the runtime. + # #__name__ is now a string key in the host. TStr may not be an actual + # CSlice in the runtime, they might be a CSlice with length = i32::MAX and + # ptr = string key in the host. # Keep this in sync with the function ARTIQIRGenerator.alloc_exn. attributes = OrderedDict([ - ("__name__", TStr()), - ("__file__", TStr()), - ("__line__", TInt32()), - ("__col__", TInt32()), - ("__func__", TStr()), - ("__message__", TStr()), - ("__param0__", TInt64()), - ("__param1__", TInt64()), - ("__param2__", TInt64()), + ("#__name__", TInt32()), + ("#__file__", TStr()), + ("#__line__", TInt32()), + ("#__col__", TInt32()), + ("#__func__", TStr()), + ("#__message__", TStr()), + ("#__param0__", TInt64()), + ("#__param1__", TInt64()), + ("#__param2__", TInt64()), ]) def __init__(self, name="Exception", id=0): diff --git a/artiq/compiler/embedding.py b/artiq/compiler/embedding.py index 89866c7c8..2a6ce62c5 100644 --- a/artiq/compiler/embedding.py +++ b/artiq/compiler/embedding.py @@ -47,6 +47,24 @@ class EmbeddingMap: self.module_map = {} self.type_map = {} self.function_map = {} + self.str_forward_map = {} + self.str_reverse_map = {} + + def preallocate_runtime_exception_names(self, names): + for i, name in enumerate(names): + exn_id = self.store_str("0:artiq.coredevice.exceptions." + name) + assert exn_id == i + + def store_str(self, s): + if s in self.str_forward_map: + return self.str_forward_map[s] + str_id = len(self.str_forward_map) + self.str_forward_map[s] = str_id + self.str_reverse_map[str_id] = s + return str_id + + def retrieve_str(self, str_id): + return self.str_reverse_map[str_id] # Modules def store_module(self, module, module_type): @@ -736,6 +754,12 @@ class Stitcher: self.functions = {} self.embedding_map = EmbeddingMap() + self.embedding_map.preallocate_runtime_exception_names(["runtimeerror", + "RTIOUnderflow", + "RTIOOverflow", + "RTIODestinationUnreachable", + "DMAError", + "I2CError"]) self.value_map = defaultdict(lambda: []) self.definitely_changed = False diff --git a/artiq/compiler/module.py b/artiq/compiler/module.py index d43404b20..7f027ba2f 100644 --- a/artiq/compiler/module.py +++ b/artiq/compiler/module.py @@ -57,7 +57,8 @@ class Module: constness_validator = validators.ConstnessValidator(engine=self.engine) artiq_ir_generator = transforms.ARTIQIRGenerator(engine=self.engine, module_name=src.name, - ref_period=ref_period) + ref_period=ref_period, + embedding_map=self.embedding_map) dead_code_eliminator = transforms.DeadCodeEliminator(engine=self.engine) local_access_validator = validators.LocalAccessValidator(engine=self.engine) local_demoter = transforms.LocalDemoter() diff --git a/artiq/compiler/transforms/artiq_ir_generator.py b/artiq/compiler/transforms/artiq_ir_generator.py index 069422d1b..7b6a1a985 100644 --- a/artiq/compiler/transforms/artiq_ir_generator.py +++ b/artiq/compiler/transforms/artiq_ir_generator.py @@ -88,8 +88,9 @@ class ARTIQIRGenerator(algorithm.Visitor): _size_type = builtins.TInt32() - def __init__(self, module_name, engine, ref_period): + def __init__(self, module_name, engine, ref_period, embedding_map): self.engine = engine + self.embedding_map = embedding_map self.functions = [] self.name = [module_name] if module_name != "" else [] self.ref_period = ir.Constant(ref_period, builtins.TFloat()) @@ -638,10 +639,10 @@ class ARTIQIRGenerator(algorithm.Visitor): loc_column = ir.Constant(loc.column(), builtins.TInt32()) loc_function = ir.Constant(".".join(self.name), builtins.TStr()) - self.append(ir.SetAttr(exn, "__file__", loc_file)) - self.append(ir.SetAttr(exn, "__line__", loc_line)) - self.append(ir.SetAttr(exn, "__col__", loc_column)) - self.append(ir.SetAttr(exn, "__func__", loc_function)) + self.append(ir.SetAttr(exn, "#__file__", loc_file)) + self.append(ir.SetAttr(exn, "#__line__", loc_line)) + self.append(ir.SetAttr(exn, "#__col__", loc_column)) + self.append(ir.SetAttr(exn, "#__func__", loc_function)) if self.unwind_target is not None: self.append(ir.Raise(exn, self.unwind_target)) @@ -2105,8 +2106,9 @@ class ARTIQIRGenerator(algorithm.Visitor): def alloc_exn(self, typ, message=None, param0=None, param1=None, param2=None): typ = typ.find() name = "{}:{}".format(typ.id, typ.name) + name_id = self.embedding_map.store_str(name) attributes = [ - ir.Constant(name, builtins.TStr()), # typeinfo + ir.Constant(name_id, builtins.TInt32()), # typeinfo ir.Constant("", builtins.TStr()), # file ir.Constant(0, builtins.TInt32()), # line ir.Constant(0, builtins.TInt32()), # column @@ -2578,10 +2580,10 @@ class ARTIQIRGenerator(algorithm.Visitor): old_unwind, self.unwind_target = self.unwind_target, None exn = self.alloc_exn(builtins.TException("AssertionError"), message=msg) - self.append(ir.SetAttr(exn, "__file__", file)) - self.append(ir.SetAttr(exn, "__line__", line)) - self.append(ir.SetAttr(exn, "__col__", col)) - self.append(ir.SetAttr(exn, "__func__", function)) + self.append(ir.SetAttr(exn, "#__file__", file)) + self.append(ir.SetAttr(exn, "#__line__", line)) + self.append(ir.SetAttr(exn, "#__col__", col)) + self.append(ir.SetAttr(exn, "#__func__", function)) self.append(ir.Raise(exn)) finally: self.current_function = old_func @@ -2717,11 +2719,11 @@ class ARTIQIRGenerator(algorithm.Visitor): format_string += ")" elif builtins.is_exception(value.type): - name = self.append(ir.GetAttr(value, "__name__")) - message = self.append(ir.GetAttr(value, "__message__")) - param1 = self.append(ir.GetAttr(value, "__param0__")) - param2 = self.append(ir.GetAttr(value, "__param1__")) - param3 = self.append(ir.GetAttr(value, "__param2__")) + name = self.append(ir.GetAttr(value, "#__name__")) + message = self.append(ir.GetAttr(value, "#__message__")) + param1 = self.append(ir.GetAttr(value, "#__param0__")) + param2 = self.append(ir.GetAttr(value, "#__param1__")) + param3 = self.append(ir.GetAttr(value, "#__param2__")) format_string += "%.*s(%.*s, %lld, %lld, %lld)" args += [name, message, param1, param2, param3] diff --git a/artiq/coredevice/comm_kernel.py b/artiq/coredevice/comm_kernel.py index 37ffd2aeb..cbe1ac6db 100644 --- a/artiq/coredevice/comm_kernel.py +++ b/artiq/coredevice/comm_kernel.py @@ -571,29 +571,33 @@ class CommKernel: self._write_header(Request.RPCException) + # Note: instead of sending strings, we send object ID + # This is to avoid the need of allocatio on the device side + # This is a special case: this only applies to exceptions if hasattr(exn, "artiq_core_exception"): exn = exn.artiq_core_exception - self._write_string(exn.name) - self._write_string(self._truncate_message(exn.message)) + self._write_int32(embedding_map.store_str(exn.name)) + self._write_int32(embedding_map.store_str(self._truncate_message(exn.message))) for index in range(3): self._write_int64(exn.param[index]) filename, line, column, function = exn.traceback[-1] - self._write_string(filename) + self._write_int32(embedding_map.store_str(filename)) self._write_int32(line) self._write_int32(column) - self._write_string(function) + self._write_int32(embedding_map.store_str(function)) else: exn_type = type(exn) if exn_type in (ZeroDivisionError, ValueError, IndexError, RuntimeError) or \ hasattr(exn, "artiq_builtin"): - self._write_string("0:{}".format(exn_type.__name__)) + name = "0:{}".format(exn_type.__name__) else: exn_id = embedding_map.store_object(exn_type) - self._write_string("{}:{}.{}".format(exn_id, - exn_type.__module__, - exn_type.__qualname__)) - self._write_string(self._truncate_message(str(exn))) + name = "{}:{}.{}".format(exn_id, + exn_type.__module__, + exn_type.__qualname__) + self._write_int32(embedding_map.store_str(name)) + self._write_int32(embedding_map.store_str(self._truncate_message(str(exn)))) for index in range(3): self._write_int64(0) @@ -604,10 +608,10 @@ class CommKernel: ((filename, line, function, _), ) = tb else: assert False - self._write_string(filename) + self._write_int32(embedding_map.store_str(filename)) self._write_int32(line) self._write_int32(-1) # column not known - self._write_string(function) + self._write_int32(embedding_map.store_str(function)) self._flush() else: logger.debug("rpc service: %d %r %r = %r", From da4ff44377512b4b2fdd61c6e0d437153662dd4a Mon Sep 17 00:00:00 2001 From: pca006132 Date: Sun, 23 Jan 2022 21:12:12 +0800 Subject: [PATCH 3/8] compiler: fixed try codegen and allocate exceptions Exceptions are now allocated in the runtime when we raise the exception, and destroyed when we exit the catch block. Nested exception and try block is now supported, and should behave the same as in CPython. Exceptions raised in except blocks will now unwind through finally blocks, matching the behavior in CPython. Reraise will now preserve backtrace. Phi block LLVM IR generation is modified to handle landingpads, which one ARTIQ IR will map to multiple LLVM IR. --- artiq/compiler/ir.py | 6 +- .../compiler/transforms/artiq_ir_generator.py | 235 ++++++++++++------ .../compiler/transforms/llvm_ir_generator.py | 130 +++++++--- 3 files changed, 261 insertions(+), 110 deletions(-) diff --git a/artiq/compiler/ir.py b/artiq/compiler/ir.py index 92bc4fb3a..8bc84e598 100644 --- a/artiq/compiler/ir.py +++ b/artiq/compiler/ir.py @@ -1245,9 +1245,9 @@ class Raise(Terminator): if len(self.operands) > 1: return self.operands[1] -class Reraise(Terminator): +class Resume(Terminator): """ - A reraise instruction. + A resume instruction. """ """ @@ -1261,7 +1261,7 @@ class Reraise(Terminator): super().__init__(operands, builtins.TNone(), name) def opcode(self): - return "reraise" + return "resume" def exception_target(self): if len(self.operands) > 0: diff --git a/artiq/compiler/transforms/artiq_ir_generator.py b/artiq/compiler/transforms/artiq_ir_generator.py index 7b6a1a985..bd69ca79c 100644 --- a/artiq/compiler/transforms/artiq_ir_generator.py +++ b/artiq/compiler/transforms/artiq_ir_generator.py @@ -8,6 +8,7 @@ semantics explicitly. from collections import OrderedDict, defaultdict from functools import reduce +from itertools import chain from pythonparser import algorithm, diagnostic, ast from .. import types, builtins, asttyped, ir, iodelay @@ -61,6 +62,9 @@ class ARTIQIRGenerator(algorithm.Visitor): the basic block to which ``return`` will transfer control :ivar unwind_target: (:class:`ir.BasicBlock` or None) the basic block to which unwinding will transfer control + :ivar catch_clauses: (list of (:class:`ir.BasicBlock`, :class:`types.Type` or None)) + a list of catch clauses that should be appended to inner try block + landingpad :ivar final_branch: (function (target: :class:`ir.BasicBlock`, block: :class:`ir.BasicBlock) or None) the function that appends to ``block`` a jump through the ``finally`` statement @@ -103,10 +107,13 @@ class ARTIQIRGenerator(algorithm.Visitor): self.current_private_env = None self.current_args = None self.current_assign = None + self.current_exception = None self.break_target = None self.continue_target = None self.return_target = None self.unwind_target = None + self.catch_clauses = [] + self.outer_final = None self.final_branch = None self.function_map = dict() self.variable_map = dict() @@ -650,9 +657,9 @@ class ARTIQIRGenerator(algorithm.Visitor): self.append(ir.Raise(exn)) else: if self.unwind_target is not None: - self.append(ir.Reraise(self.unwind_target)) + self.append(ir.Resume(self.unwind_target)) else: - self.append(ir.Reraise()) + self.append(ir.Resume()) def visit_Raise(self, node): if node.exc is not None and types.is_exn_constructor(node.exc.type): @@ -662,6 +669,9 @@ class ARTIQIRGenerator(algorithm.Visitor): def visit_Try(self, node): dispatcher = self.add_block("try.dispatch") + cleanup = self.add_block('handler.cleanup') + landingpad = ir.LandingPad(cleanup) + dispatcher.append(landingpad) if any(node.finalbody): # k for continuation @@ -677,15 +687,6 @@ class ARTIQIRGenerator(algorithm.Visitor): final_targets.append(target) final_paths.append(block) - final_exn_targets = [] - final_exn_paths = [] - # raise has to be treated differently - # we cannot follow indirectbr for local access validation, so we - # have to construct the control flow explicitly - def exception_final_branch(target, block): - final_exn_targets.append(target) - final_exn_paths.append(block) - if self.break_target is not None: break_proxy = self.add_block("try.break") old_break, self.break_target = self.break_target, break_proxy @@ -706,15 +707,52 @@ class ARTIQIRGenerator(algorithm.Visitor): return_action.append(ir.Return(value)) final_branch(return_action, return_proxy) + old_outer_final, self.outer_final = self.outer_final, final_branch + elif self.outer_final is None: + landingpad.has_cleanup = False + + # we should propagate the clauses to nested try catch blocks + # so nested try catch will jump to our clause if the inner one does not + # match + # note that the phi instruction here requires some hack, see + # llvm_ir_generator process_function for details + clauses = [] + found_catch_all = False + for handler_node in node.handlers: + if found_catch_all: + self.warn_unreachable(handler_node) + continue + exn_type = handler_node.name_type.find() + if handler_node.filter is not None and \ + not builtins.is_exception(exn_type, 'Exception'): + handler = self.add_block("handler." + exn_type.name) + phi = ir.Phi(builtins.TException(), 'exn') + handler.append(phi) + clauses.append((handler, exn_type, phi)) + else: + handler = self.add_block("handler.catchall") + phi = ir.Phi(builtins.TException(), 'exn') + handler.append(phi) + clauses.append((handler, None, phi)) + found_catch_all = True + + all_clauses = clauses[:] + for clause in self.catch_clauses: + # if the last clause is accept all, do not add further clauses + if len(all_clauses) == 0 or all_clauses[-1][1] is not None: + all_clauses.append(clause) + body = self.add_block("try.body") self.append(ir.Branch(body)) self.current_block = body + old_unwind, self.unwind_target = self.unwind_target, dispatcher + old_clauses, self.catch_clauses = self.catch_clauses, all_clauses try: - old_unwind, self.unwind_target = self.unwind_target, dispatcher self.visit(node.body) finally: self.unwind_target = old_unwind + self.catch_clauses = old_clauses if not self.current_block.is_terminated(): self.visit(node.orelse) @@ -723,95 +761,152 @@ class ARTIQIRGenerator(algorithm.Visitor): body = self.current_block if any(node.finalbody): + # if we have a final block, we should not append clauses to our + # landingpad or we will skip the finally block. + # when the finally block calls resume, it will unwind to the outer + # try catch block automatically + all_clauses = clauses + # reset targets if self.break_target: self.break_target = old_break if self.continue_target: self.continue_target = old_continue self.return_target = old_return - old_final_branch, self.final_branch = self.final_branch, exception_final_branch + if any(node.finalbody) or self.outer_final is not None: + # create new unwind target for cleanup + final_dispatcher = self.add_block("try.final.dispatch") + final_landingpad = ir.LandingPad(cleanup) + final_dispatcher.append(final_landingpad) - cleanup = self.add_block('handler.cleanup') - landingpad = dispatcher.append(ir.LandingPad(cleanup)) - if not any(node.finalbody): - landingpad.has_cleanup = False + # make sure that exception clauses are unwinded to the finally block + old_unwind, self.unwind_target = self.unwind_target, final_dispatcher + + if any(node.finalbody): + redirect = final_branch + elif self.outer_final is not None: + redirect = self.outer_final + else: + redirect = lambda dest, proxy: proxy.append(ir.Branch(dest)) + + # we need to set break/continue/return to execute end_catch + if self.break_target is not None: + break_proxy = self.add_block("try.break") + break_proxy.append(ir.Builtin("end_catch", [], builtins.TNone())) + old_break, self.break_target = self.break_target, break_proxy + redirect(old_break, break_proxy) + + if self.continue_target is not None: + continue_proxy = self.add_block("try.continue") + continue_proxy.append(ir.Builtin("end_catch", [], + builtins.TNone())) + old_continue, self.continue_target = self.continue_target, continue_proxy + redirect(old_continue, continue_proxy) + + return_proxy = self.add_block("try.return") + return_proxy.append(ir.Builtin("end_catch", [], builtins.TNone())) + old_return, self.return_target = self.return_target, return_proxy + old_return_target = old_return + if old_return_target is None: + old_return_target = self.add_block("try.doreturn") + value = old_return_target.append(ir.GetLocal(self.current_private_env, "$return")) + old_return_target.append(ir.Return(value)) + redirect(old_return_target, return_proxy) handlers = [] - for handler_node in node.handlers: - exn_type = handler_node.name_type.find() - if handler_node.filter is not None and \ - not builtins.is_exception(exn_type, 'Exception'): - handler = self.add_block("handler." + exn_type.name) - landingpad.add_clause(handler, exn_type) - else: - handler = self.add_block("handler.catchall") - landingpad.add_clause(handler, None) + for (handler_node, (handler, exn_type, phi)) in zip(node.handlers, clauses): self.current_block = handler if handler_node.name is not None: - exn = self.append(ir.Builtin("exncast", [landingpad], handler_node.name_type)) + exn = self.append(ir.Builtin("exncast", [phi], handler_node.name_type)) self._set_local(handler_node.name, exn) self.visit(handler_node.body) + # only need to call end_catch if the current block is not terminated + # other possible paths: break/continue/return/raise + # we will call end_catch in the first 3 cases, and we should not + # end_catch in the last case for nested exception + if not self.current_block.is_terminated(): + self.append(ir.Builtin("end_catch", [], builtins.TNone())) post_handler = self.current_block + handlers.append(post_handler) - handlers.append((handler, post_handler)) + # branch to all possible clauses, including those from outer try catch + # block + # if we have a finally block, all_clauses will not include those from + # the outer block + for (handler, clause, phi) in all_clauses: + phi.add_incoming(landingpad, dispatcher) + landingpad.add_clause(handler, clause) + + if self.break_target: + self.break_target = old_break + if self.continue_target: + self.continue_target = old_continue + self.return_target = old_return if any(node.finalbody): # Finalize and continue after try statement. - self.final_branch = old_final_branch - - for (i, (target, block)) in enumerate(zip(final_exn_targets, final_exn_paths)): - finalizer = self.add_block(f"finally{i}") - self.current_block = block - self.terminate(ir.Branch(finalizer)) - self.current_block = finalizer - self.visit(node.finalbody) - self.terminate(ir.Branch(target)) - - finalizer = self.add_block("finally") - self.current_block = finalizer - - self.visit(node.finalbody) - post_finalizer = self.current_block - - # Finalize and reraise. Separate from previous case to expose flow - # to LocalAccessValidator. - finalizer_reraise = self.add_block("finally.reraise") + self.outer_final = old_outer_final + self.unwind_target = old_unwind + # Exception path + finalizer_reraise = self.add_block("finally.resume") self.current_block = finalizer_reraise - self.visit(node.finalbody) - self.terminate(ir.Reraise(self.unwind_target)) - - self.current_block = tail = self.add_block("try.tail") - if any(node.finalbody): - final_targets.append(tail) - - for block in final_paths: - block.append(ir.Branch(finalizer)) - - if not body.is_terminated(): - body.append(ir.SetLocal(final_state, "$cont", tail)) - body.append(ir.Branch(finalizer)) - + self.terminate(ir.Resume(self.unwind_target)) cleanup.append(ir.Branch(finalizer_reraise)) - for handler, post_handler in handlers: - if not post_handler.is_terminated(): - post_handler.append(ir.SetLocal(final_state, "$cont", tail)) - post_handler.append(ir.Branch(finalizer)) + # Normal path + finalizer = self.add_block("finally") + self.current_block = finalizer + self.visit(node.finalbody) + post_finalizer = self.current_block + self.current_block = tail = self.add_block("try.tail") + final_targets.append(tail) + # if final block is not terminated, branch to tail if not post_finalizer.is_terminated(): dest = post_finalizer.append(ir.GetLocal(final_state, "$cont")) post_finalizer.append(ir.IndirectBranch(dest, final_targets)) + # make sure proxies will branch to finalizer + for block in final_paths: + if finalizer in block.predecessors(): + # avoid producing irreducible graphs + # generate a new finalizer + self.current_block = tmp_finalizer = self.add_block("finally.tmp") + self.visit(node.finalbody) + if not self.current_block.is_terminated(): + assert isinstance(block.instructions[-1], ir.SetLocal) + self.current_block.append(ir.Branch(block.instructions[-1].operands[-1])) + block.instructions[-1].erase() + block.append(ir.Branch(tmp_finalizer)) + self.current_block = tail + else: + block.append(ir.Branch(finalizer)) + # if no raise in body/handlers, branch to finalizer + for block in chain([body], handlers): + if not block.is_terminated(): + if finalizer in block.predecessors(): + # similar to the above case + self.current_block = tmp_finalizer = self.add_block("finally.tmp") + self.visit(node.finalbody) + self.terminate(ir.Branch(tail)) + block.append(ir.Branch(tmp_finalizer)) + self.current_block = tail + else: + block.append(ir.SetLocal(final_state, "$cont", tail)) + block.append(ir.Branch(finalizer)) else: + if self.outer_final is not None: + self.unwind_target = old_unwind + self.current_block = tail = self.add_block("try.tail") if not body.is_terminated(): body.append(ir.Branch(tail)) - cleanup.append(ir.Reraise(self.unwind_target)) + cleanup.append(ir.Resume(self.unwind_target)) - for handler, post_handler in handlers: - if not post_handler.is_terminated(): - post_handler.append(ir.Branch(tail)) + for handler in handlers: + if not handler.is_terminated(): + handler.append(ir.Branch(tail)) def _try_finally(self, body_gen, finally_gen, name): dispatcher = self.add_block("{}.dispatch".format(name)) @@ -830,7 +925,7 @@ class ARTIQIRGenerator(algorithm.Visitor): self.current_block = self.add_block("{}.cleanup".format(name)) dispatcher.append(ir.LandingPad(self.current_block)) finally_gen() - self.raise_exn() + self.terminate(ir.Resume(self.unwind_target)) self.current_block = self.post_body diff --git a/artiq/compiler/transforms/llvm_ir_generator.py b/artiq/compiler/transforms/llvm_ir_generator.py index 1a206391e..60a5dc482 100644 --- a/artiq/compiler/transforms/llvm_ir_generator.py +++ b/artiq/compiler/transforms/llvm_ir_generator.py @@ -171,11 +171,17 @@ class LLVMIRGenerator: self.llfunction = None self.llmap = {} self.llobject_map = {} + self.llpred_map = {} self.phis = [] self.debug_info_emitter = DebugInfoEmitter(self.llmodule) self.empty_metadata = self.llmodule.add_metadata([]) self.quote_fail_msg = None + def add_pred(self, pred, block): + if block not in self.llpred_map: + self.llpred_map[block] = set() + self.llpred_map[block].add(pred) + def needs_sret(self, lltyp, may_be_large=True): if isinstance(lltyp, ll.VoidType): return False @@ -367,7 +373,9 @@ class LLVMIRGenerator: llty = ll.FunctionType(lli32, [], var_arg=True) elif name == "__artiq_raise": llty = ll.FunctionType(llvoid, [self.llty_of_type(builtins.TException())]) - elif name == "__artiq_reraise": + elif name == "__artiq_resume": + llty = ll.FunctionType(llvoid, []) + elif name == "__artiq_end_catch": llty = ll.FunctionType(llvoid, []) elif name == "memcmp": llty = ll.FunctionType(lli32, [llptr, llptr, lli32]) @@ -653,6 +661,28 @@ class LLVMIRGenerator: self.llbuilder = ll.IRBuilder() llblock_map = {} + # this is the predecessor map, from basic block to the set of its + # predecessors + # handling for branch and cbranch is here, and the handling of + # indirectbr and landingpad are in their respective process_* + # function + self.llpred_map = llpred_map = {} + branch_fn = self.llbuilder.branch + cbranch_fn = self.llbuilder.cbranch + def override_branch(block): + nonlocal self, branch_fn + self.add_pred(self.llbuilder.basic_block, block) + return branch_fn(block) + + def override_cbranch(pred, bbif, bbelse): + nonlocal self, cbranch_fn + self.add_pred(self.llbuilder.basic_block, bbif) + self.add_pred(self.llbuilder.basic_block, bbelse) + return cbranch_fn(pred, bbif, bbelse) + + self.llbuilder.branch = override_branch + self.llbuilder.cbranch = override_cbranch + if not func.is_generated: lldisubprogram = self.debug_info_emitter.emit_subprogram(func, self.llfunction) self.llfunction.set_metadata('dbg', lldisubprogram) @@ -675,6 +705,10 @@ class LLVMIRGenerator: # Third, translate all instructions. for block in func.basic_blocks: self.llbuilder.position_at_end(self.llmap[block]) + old_block = None + if len(block.instructions) == 1 and \ + isinstance(block.instructions[0], ir.LandingPad): + old_block = self.llbuilder.basic_block for insn in block.instructions: if insn.loc is not None and not func.is_generated: self.llbuilder.debug_metadata = \ @@ -689,12 +723,28 @@ class LLVMIRGenerator: # instruction so that the result spans several LLVM basic # blocks. This only really matters for phis, which are thus # using a different map (the following one). - llblock_map[block] = self.llbuilder.basic_block + if old_block is None: + llblock_map[block] = self.llbuilder.basic_block + else: + llblock_map[block] = old_block # Fourth, add incoming values to phis. for phi, llphi in self.phis: for value, block in phi.incoming(): - llphi.add_incoming(self.map(value), llblock_map[block]) + if isinstance(phi.type, builtins.TException): + # a hack to patch phi from landingpad + # because landingpad is a single bb in artiq IR, but + # generates multiple bb, we need to find out the + # predecessor to figure out the actual bb + landingpad = llblock_map[block] + for pred in llpred_map[llphi.parent]: + if pred in llpred_map and landingpad in llpred_map[pred]: + llphi.add_incoming(self.map(value), pred) + break + else: + llphi.add_incoming(self.map(value), landingpad) + else: + llphi.add_incoming(self.map(value), llblock_map[block]) finally: self.function_flags = None self.llfunction = None @@ -1247,6 +1297,8 @@ class LLVMIRGenerator: return llstore_lo else: return self.llbuilder.call(self.llbuiltin("delay_mu"), [llinterval]) + elif insn.op == "end_catch": + return self.llbuilder.call(self.llbuiltin("__artiq_end_catch"), []) else: assert False @@ -1678,7 +1730,12 @@ class LLVMIRGenerator: def process_IndirectBranch(self, insn): llinsn = self.llbuilder.branch_indirect(self.map(insn.target())) for dest in insn.destinations(): - llinsn.add_destination(self.map(dest)) + dest = self.map(dest) + self.add_pred(self.llbuilder.basic_block, dest) + if dest not in self.llpred_map: + self.llpred_map[dest] = set() + self.llpred_map[dest].add(self.llbuilder.basic_block) + llinsn.add_destination(dest) return llinsn def process_Return(self, insn): @@ -1716,8 +1773,8 @@ class LLVMIRGenerator: llexn = self.map(insn.value()) return self._gen_raise(insn, self.llbuiltin("__artiq_raise"), [llexn]) - def process_Reraise(self, insn): - return self._gen_raise(insn, self.llbuiltin("__artiq_reraise"), []) + def process_Resume(self, insn): + return self._gen_raise(insn, self.llbuiltin("__artiq_resume"), []) def process_LandingPad(self, insn): # Layout on return from landing pad: {%_Unwind_Exception*, %Exception*} @@ -1726,10 +1783,11 @@ class LLVMIRGenerator: cleanup=insn.has_cleanup) llrawexn = self.llbuilder.extract_value(lllandingpad, 1) llexn = self.llbuilder.bitcast(llrawexn, self.llty_of_type(insn.type)) - llexnnameptr = self.llbuilder.gep(llexn, [self.llindex(0), self.llindex(0)], + llexnidptr = self.llbuilder.gep(llexn, [self.llindex(0), self.llindex(0)], inbounds=True) - llexnname = self.llbuilder.load(llexnnameptr) + llexnid = self.llbuilder.load(llexnidptr) + landingpadbb = self.llbuilder.basic_block for target, typ in insn.clauses(): if typ is None: # we use a null pointer here, similar to how cpp does it @@ -1742,42 +1800,40 @@ class LLVMIRGenerator: ll.Constant(lli32, 0).inttoptr(llptr) ) ) - else: - exnname = "{}:{}".format(typ.id, typ.name) - llclauseexnname = self.llconst_of_const( - ir.Constant(exnname, builtins.TStr())) - llclauseexnnameptr = self.llmodule.globals.get("exn.{}".format(exnname)) - if llclauseexnnameptr is None: - llclauseexnnameptr = ll.GlobalVariable(self.llmodule, llclauseexnname.type, - name="exn.{}".format(exnname)) - llclauseexnnameptr.global_constant = True - llclauseexnnameptr.initializer = llclauseexnname - llclauseexnnameptr.linkage = "private" - llclauseexnnameptr.unnamed_addr = True - lllandingpad.add_clause(ll.CatchClause(llclauseexnnameptr)) - - if typ is None: # typ is None means that we match all exceptions, so no need to # compare - self.llbuilder.branch(self.map(target)) + target = self.map(target) + self.add_pred(landingpadbb, target) + self.add_pred(landingpadbb, self.llbuilder.basic_block) + self.llbuilder.branch(target) else: - llexnlen = self.llbuilder.extract_value(llexnname, 1) - llclauseexnlen = self.llbuilder.extract_value(llclauseexnname, 1) - llmatchinglen = self.llbuilder.icmp_unsigned('==', llexnlen, llclauseexnlen) - with self.llbuilder.if_then(llmatchinglen): - llexnptr = self.llbuilder.extract_value(llexnname, 0) - llclauseexnptr = self.llbuilder.extract_value(llclauseexnname, 0) - llcomparedata = self.llbuilder.call(self.llbuiltin("memcmp"), - [llexnptr, llclauseexnptr, llexnlen]) - llmatchingdata = self.llbuilder.icmp_unsigned('==', llcomparedata, - ll.Constant(lli32, 0)) - with self.llbuilder.if_then(llmatchingdata): - self.llbuilder.branch(self.map(target)) + exnname = "{}:{}".format(typ.id, typ.name) + llclauseexnidptr = self.llmodule.globals.get("exn.{}".format(exnname)) + exnid = ll.Constant(lli32, self.embedding_map.store_str(exnname)) + if llclauseexnidptr is None: + llclauseexnidptr = ll.GlobalVariable(self.llmodule, lli32, + name="exn.{}".format(exnname)) + llclauseexnidptr.global_constant = True + llclauseexnidptr.initializer = exnid + llclauseexnidptr.linkage = "private" + llclauseexnidptr.unnamed_addr = True + lllandingpad.add_clause(ll.CatchClause(llclauseexnidptr)) + llmatchingdata = self.llbuilder.icmp_unsigned("==", llexnid, + exnid) + with self.llbuilder.if_then(llmatchingdata): + target = self.map(target) + self.add_pred(landingpadbb, target) + self.add_pred(landingpadbb, self.llbuilder.basic_block) + self.llbuilder.branch(target) + self.add_pred(landingpadbb, self.llbuilder.basic_block) if self.llbuilder.basic_block.terminator is None: if insn.has_cleanup: - self.llbuilder.branch(self.map(insn.cleanup())) + target = self.map(insn.cleanup()) + self.add_pred(landingpadbb, target) + self.add_pred(landingpadbb, self.llbuilder.basic_block) + self.llbuilder.branch(target) else: self.llbuilder.resume(lllandingpad) From 6ec003c1c974d4aa5ae137b792f4db94ec10d9d9 Mon Sep 17 00:00:00 2001 From: pca006132 Date: Sun, 23 Jan 2022 21:16:19 +0800 Subject: [PATCH 4/8] compiler: fixed dead code eliminator Instead of removing basic blocks with no predecessor, we will now mark and remove all blocks that are unreachable from the entry block. This can handle loops that are dead code. This is needed as we will now generate more complicated code for exception handling which the old dead code eliminator failed to handle. --- artiq/compiler/ir.py | 13 ++------ .../transforms/dead_code_eliminator.py | 31 ++++++++++++++----- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/artiq/compiler/ir.py b/artiq/compiler/ir.py index 8bc84e598..88ef3a151 100644 --- a/artiq/compiler/ir.py +++ b/artiq/compiler/ir.py @@ -135,6 +135,7 @@ class NamedValue(Value): def __init__(self, typ, name): super().__init__(typ) self.name, self.function = name, None + self.is_removed = False def set_name(self, new_name): if self.function is not None: @@ -235,7 +236,7 @@ class Instruction(User): self.drop_references() # Check this after drop_references in case this # is a self-referencing phi. - assert not any(self.uses) + assert all(use.is_removed for use in self.uses) def replace_with(self, value): self.replace_all_uses_with(value) @@ -370,7 +371,7 @@ class BasicBlock(NamedValue): self.remove_from_parent() # Check this after erasing instructions in case the block # loops into itself. - assert not any(self.uses) + assert all(use.is_removed for use in self.uses) def prepend(self, insn): assert isinstance(insn, Instruction) @@ -1360,14 +1361,6 @@ class LandingPad(Terminator): def cleanup(self): return self.operands[0] - def erase(self): - self.remove_from_parent() - # we should erase all clauses as well - for block in set(self.operands): - block.uses.remove(self) - block.erase() - assert not any(self.uses) - def clauses(self): return zip(self.operands[1:], self.types) diff --git a/artiq/compiler/transforms/dead_code_eliminator.py b/artiq/compiler/transforms/dead_code_eliminator.py index 608a46d55..f4862f2f9 100644 --- a/artiq/compiler/transforms/dead_code_eliminator.py +++ b/artiq/compiler/transforms/dead_code_eliminator.py @@ -15,13 +15,26 @@ class DeadCodeEliminator: self.process_function(func) def process_function(self, func): - modified = True - while modified: - modified = False - for block in list(func.basic_blocks): - if not any(block.predecessors()) and block != func.entry(): - self.remove_block(block) - modified = True + # defer removing those blocks, so our use checks will ignore deleted blocks + preserve = [func.entry()] + work_list = [func.entry()] + while any(work_list): + block = work_list.pop() + for succ in block.successors(): + if succ not in preserve: + preserve.append(succ) + work_list.append(succ) + + to_be_removed = [] + for block in func.basic_blocks: + if block not in preserve: + block.is_removed = True + to_be_removed.append(block) + for insn in block.instructions: + insn.is_removed = True + + for block in to_be_removed: + self.remove_block(block) modified = True while modified: @@ -42,6 +55,8 @@ class DeadCodeEliminator: def remove_block(self, block): # block.uses are updated while iterating for use in set(block.uses): + if use.is_removed: + continue if isinstance(use, ir.Phi): use.remove_incoming_block(block) if not any(use.operands): @@ -56,6 +71,8 @@ class DeadCodeEliminator: def remove_instruction(self, insn): for use in set(insn.uses): + if use.is_removed: + continue if isinstance(use, ir.Phi): use.remove_incoming_value(insn) if not any(use.operands): From ba34700798fbcdc80b5afc1bbac0f8f84e6d739d Mon Sep 17 00:00:00 2001 From: pca006132 Date: Sun, 23 Jan 2022 21:19:54 +0800 Subject: [PATCH 5/8] coredevice: report nested exceptions --- artiq/compiler/targets.py | 10 ++- artiq/coredevice/comm_kernel.py | 55 +++++++++++---- artiq/coredevice/exceptions.py | 116 ++++++++++++++++++++++---------- 3 files changed, 132 insertions(+), 49 deletions(-) diff --git a/artiq/compiler/targets.py b/artiq/compiler/targets.py index dc5b68be8..20543b6a6 100644 --- a/artiq/compiler/targets.py +++ b/artiq/compiler/targets.py @@ -212,6 +212,7 @@ class Target: # just after the call. Offset them back to get an address somewhere # inside the call instruction (or its delay slot), since that's what # the backtrace entry should point at. + last_inlined = None offset_addresses = [hex(addr - 1) for addr in addresses] with RunTool([self.tool_addr2line, "--addresses", "--functions", "--inlines", "--demangle", "--exe={library}"] + offset_addresses, @@ -227,9 +228,11 @@ class Target: if address_or_function[:2] == "0x": address = int(address_or_function[2:], 16) + 1 # remove offset function = next(lines) + inlined = False else: address = backtrace[-1][4] # inlined function = address_or_function + inlined = True location = next(lines) filename, line = location.rsplit(":", 1) @@ -240,7 +243,12 @@ class Target: else: line = int(line) # can't get column out of addr2line D: - backtrace.append((filename, line, -1, function, address)) + if inlined: + last_inlined.append((filename, line, -1, function, address)) + else: + last_inlined = [] + backtrace.append((filename, line, -1, function, address, + last_inlined)) return backtrace def demangle(self, names): diff --git a/artiq/coredevice/comm_kernel.py b/artiq/coredevice/comm_kernel.py index cbe1ac6db..e6551cb0a 100644 --- a/artiq/coredevice/comm_kernel.py +++ b/artiq/coredevice/comm_kernel.py @@ -623,28 +623,59 @@ class CommKernel: self._flush() def _serve_exception(self, embedding_map, symbolizer, demangler): - name = self._read_string() - message = self._read_string() - params = [self._read_int64() for _ in range(3)] + exception_count = self._read_int32() + nested_exceptions = [] - filename = self._read_string() - line = self._read_int32() - column = self._read_int32() - function = self._read_string() + def read_exception_string(): + # note: if length == -1, the following int32 is the object key + length = self._read_int32() + if length == -1: + return embedding_map.retrieve_str(self._read_int32()) + else: + return self._read(length).decode("utf-8") + + for _ in range(exception_count): + name = embedding_map.retrieve_str(self._read_int32()) + message = read_exception_string() + params = [self._read_int64() for _ in range(3)] + + filename = read_exception_string() + line = self._read_int32() + column = self._read_int32() + function = read_exception_string() + nested_exceptions.append([name, message, params, + filename, line, column, function]) + + demangled_names = demangler([ex[6] for ex in nested_exceptions]) + for i in range(exception_count): + nested_exceptions[i][6] = demangled_names[i] + + exception_info = [] + for _ in range(exception_count): + sp = self._read_int32() + initial_backtrace = self._read_int32() + current_backtrace = self._read_int32() + exception_info.append((sp, initial_backtrace, current_backtrace)) + + backtrace = [] + stack_pointers = [] + for _ in range(self._read_int32()): + backtrace.append(self._read_int32()) + stack_pointers.append(self._read_int32()) - backtrace = [self._read_int32() for _ in range(self._read_int32())] self._process_async_error() - traceback = list(reversed(symbolizer(backtrace))) + \ - [(filename, line, column, *demangler([function]), None)] - core_exn = exceptions.CoreException(name, message, params, traceback) + traceback = list(symbolizer(backtrace)) + core_exn = exceptions.CoreException(nested_exceptions, exception_info, + traceback, stack_pointers) if core_exn.id == 0: python_exn_type = getattr(exceptions, core_exn.name.split('.')[-1]) else: python_exn_type = embedding_map.retrieve_object(core_exn.id) - python_exn = python_exn_type(message.format(*params)) + python_exn = python_exn_type( + nested_exceptions[-1][1].format(*nested_exceptions[0][2])) python_exn.artiq_core_exception = core_exn raise python_exn diff --git a/artiq/coredevice/exceptions.py b/artiq/coredevice/exceptions.py index cfa2ce85a..9600bdd29 100644 --- a/artiq/coredevice/exceptions.py +++ b/artiq/coredevice/exceptions.py @@ -16,50 +16,94 @@ AssertionError = builtins.AssertionError class CoreException: """Information about an exception raised or passed through the core device.""" + def __init__(self, exceptions, exception_info, traceback, stack_pointers): + self.exceptions = exceptions + self.exception_info = exception_info + self.traceback = list(traceback) + self.stack_pointers = stack_pointers - def __init__(self, name, message, params, traceback): + first_exception = exceptions[0] + name = first_exception[0] if ':' in name: exn_id, self.name = name.split(':', 2) self.id = int(exn_id) else: self.id, self.name = 0, name - self.message, self.params = message, params - self.traceback = list(traceback) + self.message = first_exception[1] + self.params = first_exception[2] + + def append_backtrace(self, record, inlined=False): + filename, line, column, function, address = record + stub_globals = {"__name__": filename, "__loader__": source_loader} + source_line = linecache.getline(filename, line, stub_globals) + indentation = re.search(r"^\s*", source_line).end() + + if address is None: + formatted_address = "" + elif inlined: + formatted_address = " (inlined)" + else: + formatted_address = " (RA=+0x{:x})".format(address) + + filename = filename.replace(artiq_dir, "") + lines = [] + if column == -1: + lines.append(" {}".format(source_line.strip() if source_line else "")) + lines.append(" File \"{file}\", line {line}, in {function}{address}". + format(file=filename, line=line, function=function, + address=formatted_address)) + else: + lines.append(" {}^".format(" " * (column - indentation))) + lines.append(" {}".format(source_line.strip() if source_line else "")) + lines.append(" File \"{file}\", line {line}, column {column}," + " in {function}{address}". + format(file=filename, line=line, column=column + 1, + function=function, address=formatted_address)) + return lines + + def single_traceback(self, exception_index): + # note that we insert in reversed order + lines = [] + last_sp = 0 + start_backtrace_index = self.exception_info[exception_index][1] + zipped = list(zip(self.traceback[start_backtrace_index:], + self.stack_pointers[start_backtrace_index:])) + exception = self.exceptions[exception_index] + name = exception[0] + message = exception[1] + params = exception[2] + if ':' in name: + exn_id, name = name.split(':', 2) + exn_id = int(exn_id) + else: + exn_id = 0 + lines.append("{}({}): {}".format(name, exn_id, message.format(*params))) + zipped.append(((exception[3], exception[4], exception[5], exception[6], + None, []), None)) + + for ((filename, line, column, function, address, inlined), sp) in zipped: + # backtrace of nested exceptions may be discontinuous + # but the stack pointer must increase monotonically + if sp is not None and sp <= last_sp: + continue + last_sp = sp + + for record in reversed(inlined): + lines += self.append_backtrace(record, True) + lines += self.append_backtrace((filename, line, column, function, + address)) + + lines.append("Traceback (most recent call first):") + + return "\n".join(reversed(lines)) def __str__(self): - lines = [] - lines.append("Core Device Traceback (most recent call last):") - last_address = 0 - for (filename, line, column, function, address) in self.traceback: - stub_globals = {"__name__": filename, "__loader__": source_loader} - source_line = linecache.getline(filename, line, stub_globals) - indentation = re.search(r"^\s*", source_line).end() - - if address is None: - formatted_address = "" - elif address == last_address: - formatted_address = " (inlined)" - else: - formatted_address = " (RA=+0x{:x})".format(address) - last_address = address - - filename = filename.replace(artiq_dir, "") - if column == -1: - lines.append(" File \"{file}\", line {line}, in {function}{address}". - format(file=filename, line=line, function=function, - address=formatted_address)) - lines.append(" {}".format(source_line.strip() if source_line else "")) - else: - lines.append(" File \"{file}\", line {line}, column {column}," - " in {function}{address}". - format(file=filename, line=line, column=column + 1, - function=function, address=formatted_address)) - lines.append(" {}".format(source_line.strip() if source_line else "")) - lines.append(" {}^".format(" " * (column - indentation))) - - lines.append("{}({}): {}".format(self.name, self.id, - self.message.format(*self.params))) - return "\n".join(lines) + tracebacks = [self.single_traceback(i) for i in range(len(self.exceptions))] + traceback_str = ('\n\nDuring handling of the above exception, ' + + 'another exception occurred:\n\n').join(tracebacks) + return 'Core Device Traceback:\n' +\ + traceback_str +\ + '\n\nEnd of Code Device Traceback\n' class InternalError(Exception): From 536b3e0c26fbc93b867443db07911e8caf48f789 Mon Sep 17 00:00:00 2001 From: pca006132 Date: Sun, 23 Jan 2022 21:20:20 +0800 Subject: [PATCH 6/8] test: added test case for nested exceptions and try --- artiq/test/coredevice/test_portability.py | 63 +++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/artiq/test/coredevice/test_portability.py b/artiq/test/coredevice/test_portability.py index a861b6833..21f73000a 100644 --- a/artiq/test/coredevice/test_portability.py +++ b/artiq/test/coredevice/test_portability.py @@ -125,6 +125,57 @@ class _Pulses(EnvExperiment): class _MyException(Exception): pass +class _NestedFinally(EnvExperiment): + def build(self, trace): + self.setattr_device("core") + self.trace = trace + + def _trace(self, i): + self.trace.append(i) + + @kernel + def run(self): + try: + try: + raise ValueError + finally: + try: + raise IndexError() + except ValueError: + self._trace(0) + except: + self._trace(1) + finally: + self._trace(2) + +class _NestedExceptions(EnvExperiment): + def build(self, trace): + self.setattr_device("core") + self.trace = trace + + def _trace(self, i): + self.trace.append(i) + + @kernel + def run(self): + try: + try: + raise ValueError + except _MyException: + self._trace(0) + raise + finally: + try: + raise IndexError() + except ValueError: + self._trace(1) + raise + except IndexError: + self._trace(2) + except: + self._trace(3) + finally: + self._trace(4) class _Exceptions(EnvExperiment): def build(self, trace): @@ -253,6 +304,18 @@ class HostVsDeviceCase(ExperimentCase): _run_on_host(_Exceptions, trace=t_host) self.assertEqual(t_device, t_host) + def test_nested_finally(self): + t_device, t_host = [], [] + self.execute(_NestedFinally, trace=t_device) + _run_on_host(_NestedFinally, trace=t_host) + self.assertEqual(t_device, t_host) + + def test_nested_exceptions(self): + t_device, t_host = [], [] + self.execute(_NestedExceptions, trace=t_device) + _run_on_host(_NestedExceptions, trace=t_host) + self.assertEqual(t_device, t_host) + def test_rpc_exceptions(self): for f in self.execute, _run_on_host: with self.assertRaises(_MyException): From 4132c450a5c5294b2f177aa684816f379332286a Mon Sep 17 00:00:00 2001 From: pca006132 Date: Tue, 25 Jan 2022 15:10:09 +0800 Subject: [PATCH 7/8] firmware: runtime changes for exception Ported from: https://git.m-labs.hk/M-Labs/artiq-zynq/pulls/162 This includes new API for exception handling, some refactoring to avoid code duplication for exception structures, and modified protocols to send nested exceptions and avoid string allocation. --- artiq/firmware/Cargo.lock | 1 + artiq/firmware/ksupport/api.rs | 3 +- artiq/firmware/ksupport/eh_artiq.rs | 386 ++++++++++++------ artiq/firmware/ksupport/lib.rs | 40 +- artiq/firmware/libeh/dwarf.rs | 19 +- artiq/firmware/libeh/eh_artiq.rs | 47 +++ artiq/firmware/libeh/eh_rust.rs | 2 +- artiq/firmware/libeh/lib.rs | 1 + artiq/firmware/libproto_artiq/Cargo.toml | 1 + artiq/firmware/libproto_artiq/kernel_proto.rs | 18 +- artiq/firmware/libproto_artiq/lib.rs | 1 + .../firmware/libproto_artiq/session_proto.rs | 81 ++-- artiq/firmware/runtime/session.rs | 42 +- artiq/test/libartiq_support/lib.rs | 32 +- flake.nix | 2 +- 15 files changed, 433 insertions(+), 243 deletions(-) create mode 100644 artiq/firmware/libeh/eh_artiq.rs diff --git a/artiq/firmware/Cargo.lock b/artiq/firmware/Cargo.lock index 77d262538..e5d303a00 100644 --- a/artiq/firmware/Cargo.lock +++ b/artiq/firmware/Cargo.lock @@ -253,6 +253,7 @@ dependencies = [ "byteorder", "cslice", "dyld", + "eh", "failure", "failure_derive", "io", diff --git a/artiq/firmware/ksupport/api.rs b/artiq/firmware/ksupport/api.rs index 7b219562c..c085aa08b 100644 --- a/artiq/firmware/ksupport/api.rs +++ b/artiq/firmware/ksupport/api.rs @@ -117,7 +117,8 @@ static mut API: &'static [(&'static str, *const ())] = &[ api!(_Unwind_Resume = ::unwind::_Unwind_Resume), api!(__artiq_personality = ::eh_artiq::personality), api!(__artiq_raise = ::eh_artiq::raise), - api!(__artiq_reraise = ::eh_artiq::reraise), + api!(__artiq_resume = ::eh_artiq::resume), + api!(__artiq_end_catch = ::eh_artiq::end_catch), /* proxified syscalls */ api!(core_log), diff --git a/artiq/firmware/ksupport/eh_artiq.rs b/artiq/firmware/ksupport/eh_artiq.rs index 82be0aa4e..30e173dc0 100644 --- a/artiq/firmware/ksupport/eh_artiq.rs +++ b/artiq/firmware/ksupport/eh_artiq.rs @@ -10,12 +10,15 @@ // except according to those terms. #![allow(non_camel_case_types)] -use core::{ptr, mem}; -use cslice::CSlice; +use core::mem; +use cslice::AsCSlice; use unwind as uw; use libc::{c_int, c_void}; -use eh::dwarf::{self, EHAction, EHContext}; +use eh::{self, dwarf::{self, EHAction, EHContext}}; + +pub type Exception<'a> = eh::eh_artiq::Exception<'a>; +pub type StackPointerBacktrace = eh::eh_artiq::StackPointerBacktrace; type _Unwind_Stop_Fn = extern "C" fn(version: c_int, actions: uw::_Unwind_Action, @@ -30,40 +33,74 @@ extern { stop_parameter: *mut c_void) -> uw::_Unwind_Reason_Code; } -#[repr(C)] -#[derive(Clone, Copy)] -pub struct Exception<'a> { - pub name: CSlice<'a, u8>, - pub file: CSlice<'a, u8>, - pub line: u32, - pub column: u32, - pub function: CSlice<'a, u8>, - pub message: CSlice<'a, u8>, - pub param: [i64; 3] -} +pub static mut PAYLOAD_ADDRESS: usize = 0; const EXCEPTION_CLASS: uw::_Unwind_Exception_Class = 0x4d_4c_42_53_41_52_54_51; /* 'MLBSARTQ' */ +const MAX_INFLIGHT_EXCEPTIONS: usize = 10; const MAX_BACKTRACE_SIZE: usize = 128; -#[repr(C)] -struct ExceptionInfo { - uw_exception: uw::_Unwind_Exception, - exception: Option>, - handled: bool, - backtrace: [usize; MAX_BACKTRACE_SIZE], - backtrace_size: usize +struct ExceptionBuffer { + // we need n _Unwind_Exception, because each will have their own private data + uw_exceptions: [uw::_Unwind_Exception; MAX_INFLIGHT_EXCEPTIONS], + exceptions: [Option>; MAX_INFLIGHT_EXCEPTIONS + 1], + exception_stack: [isize; MAX_INFLIGHT_EXCEPTIONS + 1], + // nested exceptions will share the backtrace buffer, treated as a tree + // backtrace contains a tuple of IP and SP + backtrace: [(usize, usize); MAX_BACKTRACE_SIZE], + backtrace_size: usize, + // stack pointers are stored to reconstruct backtrace for each exception + stack_pointers: [StackPointerBacktrace; MAX_INFLIGHT_EXCEPTIONS + 1], + // current allocated nested exceptions + exception_count: usize, +} + +static mut EXCEPTION_BUFFER: ExceptionBuffer = ExceptionBuffer { + uw_exceptions: [uw::_Unwind_Exception { + exception_class: EXCEPTION_CLASS, + exception_cleanup: cleanup, + private: [0; uw::unwinder_private_data_size], + }; MAX_INFLIGHT_EXCEPTIONS], + exceptions: [None; MAX_INFLIGHT_EXCEPTIONS + 1], + exception_stack: [-1; MAX_INFLIGHT_EXCEPTIONS + 1], + backtrace: [(0, 0); MAX_BACKTRACE_SIZE], + backtrace_size: 0, + stack_pointers: [StackPointerBacktrace { + stack_pointer: 0, + initial_backtrace_size: 0, + current_backtrace_size: 0 + }; MAX_INFLIGHT_EXCEPTIONS + 1], + exception_count: 0 +}; + +pub unsafe extern fn reset_exception_buffer(payload_addr: usize) { + EXCEPTION_BUFFER.uw_exceptions = [uw::_Unwind_Exception { + exception_class: EXCEPTION_CLASS, + exception_cleanup: cleanup, + private: [0; uw::unwinder_private_data_size], + }; MAX_INFLIGHT_EXCEPTIONS]; + EXCEPTION_BUFFER.exceptions = [None; MAX_INFLIGHT_EXCEPTIONS + 1]; + EXCEPTION_BUFFER.exception_stack = [-1; MAX_INFLIGHT_EXCEPTIONS + 1]; + EXCEPTION_BUFFER.backtrace_size = 0; + EXCEPTION_BUFFER.exception_count = 0; + PAYLOAD_ADDRESS = payload_addr; } #[cfg(target_arch = "x86_64")] const UNWIND_DATA_REG: (i32, i32) = (0, 1); // RAX, RDX +#[cfg(target_arch = "x86_64")] +// actually this is not the SP, but frame pointer +// but it serves its purpose, and getting SP will somehow cause segfault... +const UNW_FP_REG: c_int = 12; #[cfg(any(target_arch = "riscv32"))] const UNWIND_DATA_REG: (i32, i32) = (10, 11); // X10, X11 +#[cfg(any(target_arch = "riscv32"))] +const UNW_FP_REG: c_int = 2; #[export_name="__artiq_personality"] pub extern fn personality(version: c_int, - actions: uw::_Unwind_Action, + _actions: uw::_Unwind_Action, uw_exception_class: uw::_Unwind_Exception_Class, uw_exception: *mut uw::_Unwind_Exception, context: *mut uw::_Unwind_Context) @@ -85,133 +122,236 @@ pub extern fn personality(version: c_int, get_data_start: &|| uw::_Unwind_GetDataRelBase(context), }; - let exception_info = &mut *(uw_exception as *mut ExceptionInfo); - let exception = &exception_info.exception.unwrap(); + let index = EXCEPTION_BUFFER.exception_stack[EXCEPTION_BUFFER.exception_count - 1]; + assert!(index != -1); + let exception = EXCEPTION_BUFFER.exceptions[index as usize].as_ref().unwrap(); + let id = exception.id; - let name_ptr = exception.name.as_ptr(); - let len = exception.name.len(); - let eh_action = match dwarf::find_eh_action(lsda, &eh_context, name_ptr, len) { + let eh_action = match dwarf::find_eh_action(lsda, &eh_context, id) { Ok(action) => action, Err(_) => return uw::_URC_FATAL_PHASE1_ERROR, }; - if actions as u32 & uw::_UA_SEARCH_PHASE as u32 != 0 { - match eh_action { - EHAction::None | - EHAction::Cleanup(_) => return uw::_URC_CONTINUE_UNWIND, - EHAction::Catch(_) => return uw::_URC_HANDLER_FOUND, - EHAction::Terminate => return uw::_URC_FATAL_PHASE1_ERROR, - } - } else { - match eh_action { - EHAction::None => return uw::_URC_CONTINUE_UNWIND, - EHAction::Cleanup(lpad) | - EHAction::Catch(lpad) => { - if actions as u32 & uw::_UA_HANDLER_FRAME as u32 != 0 { - exception_info.handled = true - } - - // Pass a pair of the unwinder exception and ARTIQ exception - // (which immediately follows). - uw::_Unwind_SetGR(context, UNWIND_DATA_REG.0, - uw_exception as uw::_Unwind_Word); - uw::_Unwind_SetGR(context, UNWIND_DATA_REG.1, - exception as *const _ as uw::_Unwind_Word); - uw::_Unwind_SetIP(context, lpad); - return uw::_URC_INSTALL_CONTEXT; - } - EHAction::Terminate => return uw::_URC_FATAL_PHASE2_ERROR, + match eh_action { + EHAction::None => return uw::_URC_CONTINUE_UNWIND, + EHAction::Cleanup(lpad) | + EHAction::Catch(lpad) => { + // Pass a pair of the unwinder exception and ARTIQ exception + // (which immediately follows). + uw::_Unwind_SetGR(context, UNWIND_DATA_REG.0, + uw_exception as uw::_Unwind_Word); + uw::_Unwind_SetGR(context, UNWIND_DATA_REG.1, + exception as *const _ as uw::_Unwind_Word); + uw::_Unwind_SetIP(context, lpad); + return uw::_URC_INSTALL_CONTEXT; } + EHAction::Terminate => return uw::_URC_FATAL_PHASE2_ERROR, } } } +#[export_name="__artiq_raise"] +#[unwind(allowed)] +pub unsafe extern fn raise(exception: *const Exception) -> ! { + let count = EXCEPTION_BUFFER.exception_count; + let stack = &mut EXCEPTION_BUFFER.exception_stack; + let diff = exception as isize - EXCEPTION_BUFFER.exceptions.as_ptr() as isize; + if 0 <= diff && diff <= (mem::size_of::>() * MAX_INFLIGHT_EXCEPTIONS) as isize { + let index = diff / (mem::size_of::>() as isize); + let mut found = false; + for i in 0..=MAX_INFLIGHT_EXCEPTIONS + 1 { + if found { + if stack[i] == -1 { + stack[i - 1] = index; + assert!(i == count); + break; + } else { + stack[i - 1] = stack[i]; + } + } else { + if stack[i] == index { + found = true; + } + } + } + assert!(found); + let _result = _Unwind_ForcedUnwind(&mut EXCEPTION_BUFFER.uw_exceptions[stack[count - 1] as usize], + stop_fn, core::ptr::null_mut()); + } else { + if count < MAX_INFLIGHT_EXCEPTIONS { + let exception = &*exception; + for (i, slot) in EXCEPTION_BUFFER.exceptions.iter_mut().enumerate() { + // we should always be able to find a slot + if slot.is_none() { + *slot = Some( + *mem::transmute::<*const Exception, *const Exception<'static>> + (exception)); + EXCEPTION_BUFFER.exception_stack[count] = i as isize; + EXCEPTION_BUFFER.uw_exceptions[i].private = + [0; uw::unwinder_private_data_size]; + EXCEPTION_BUFFER.stack_pointers[i] = StackPointerBacktrace { + stack_pointer: 0, + initial_backtrace_size: EXCEPTION_BUFFER.backtrace_size, + current_backtrace_size: 0, + }; + EXCEPTION_BUFFER.exception_count += 1; + let _result = _Unwind_ForcedUnwind(&mut EXCEPTION_BUFFER.uw_exceptions[i], + stop_fn, core::ptr::null_mut()); + } + } + } else { + // TODO: better reporting? + let exception = Exception { + id: get_exception_id("RuntimeError"), + file: file!().as_c_slice(), + line: line!(), + column: column!(), + // https://github.com/rust-lang/rfcs/pull/1719 + function: "__artiq_raise".as_c_slice(), + message: "too many nested exceptions".as_c_slice(), + param: [0, 0, 0] + }; + EXCEPTION_BUFFER.exceptions[MAX_INFLIGHT_EXCEPTIONS] = Some(mem::transmute(exception)); + EXCEPTION_BUFFER.stack_pointers[MAX_INFLIGHT_EXCEPTIONS] = Default::default(); + EXCEPTION_BUFFER.exception_count += 1; + uncaught_exception() + } + } + unreachable!(); +} + + +#[export_name="__artiq_resume"] +#[unwind(allowed)] +pub unsafe extern fn resume() -> ! { + assert!(EXCEPTION_BUFFER.exception_count != 0); + let i = EXCEPTION_BUFFER.exception_stack[EXCEPTION_BUFFER.exception_count - 1]; + assert!(i != -1); + let _result = _Unwind_ForcedUnwind(&mut EXCEPTION_BUFFER.uw_exceptions[i as usize], + stop_fn, core::ptr::null_mut()); + unreachable!() +} + +#[export_name="__artiq_end_catch"] +#[unwind(allowed)] +pub unsafe extern fn end_catch() { + let mut count = EXCEPTION_BUFFER.exception_count; + assert!(count != 0); + // we remove all exceptions with SP <= current exception SP + // i.e. the outer exception escapes the finally block + let index = EXCEPTION_BUFFER.exception_stack[count - 1] as usize; + EXCEPTION_BUFFER.exception_stack[count - 1] = -1; + EXCEPTION_BUFFER.exceptions[index] = None; + let outer_sp = EXCEPTION_BUFFER.stack_pointers + [index].stack_pointer; + count -= 1; + for i in (0..count).rev() { + let index = EXCEPTION_BUFFER.exception_stack[i]; + assert!(index != -1); + let index = index as usize; + let sp = EXCEPTION_BUFFER.stack_pointers[index].stack_pointer; + if sp >= outer_sp { + break; + } + EXCEPTION_BUFFER.exceptions[index] = None; + EXCEPTION_BUFFER.exception_stack[i] = -1; + count -= 1; + } + EXCEPTION_BUFFER.exception_count = count; + EXCEPTION_BUFFER.backtrace_size = if count > 0 { + let index = EXCEPTION_BUFFER.exception_stack[count - 1]; + assert!(index != -1); + EXCEPTION_BUFFER.stack_pointers[index as usize].current_backtrace_size + } else { + 0 + }; +} + extern fn cleanup(_unwind_code: uw::_Unwind_Reason_Code, - uw_exception: *mut uw::_Unwind_Exception) { - unsafe { - let exception_info = &mut *(uw_exception as *mut ExceptionInfo); + _uw_exception: *mut uw::_Unwind_Exception) { + unimplemented!() +} - exception_info.exception = None; +fn uncaught_exception() -> ! { + unsafe { + // dump way to reorder the stack + for i in 0..EXCEPTION_BUFFER.exception_count { + if EXCEPTION_BUFFER.exception_stack[i] != i as isize { + // find the correct index + let index = EXCEPTION_BUFFER.exception_stack + .iter() + .position(|v| *v == i as isize).unwrap(); + let a = EXCEPTION_BUFFER.exception_stack[index]; + let b = EXCEPTION_BUFFER.exception_stack[i]; + assert!(a != -1 && b != -1); + core::mem::swap(&mut EXCEPTION_BUFFER.exception_stack[index], + &mut EXCEPTION_BUFFER.exception_stack[i]); + core::mem::swap(&mut EXCEPTION_BUFFER.exceptions[a as usize], + &mut EXCEPTION_BUFFER.exceptions[b as usize]); + core::mem::swap(&mut EXCEPTION_BUFFER.stack_pointers[a as usize], + &mut EXCEPTION_BUFFER.stack_pointers[b as usize]); + } + } + } + unsafe { + ::terminate( + EXCEPTION_BUFFER.exceptions[..EXCEPTION_BUFFER.exception_count].as_ref(), + EXCEPTION_BUFFER.stack_pointers[..EXCEPTION_BUFFER.exception_count].as_ref(), + EXCEPTION_BUFFER.backtrace[..EXCEPTION_BUFFER.backtrace_size].as_mut()) } } -extern fn uncaught_exception(_version: c_int, - actions: uw::_Unwind_Action, - _uw_exception_class: uw::_Unwind_Exception_Class, - uw_exception: *mut uw::_Unwind_Exception, - context: *mut uw::_Unwind_Context, - _stop_parameter: *mut c_void) - -> uw::_Unwind_Reason_Code { + +// stop function which would be executed when we unwind each frame +extern fn stop_fn(_version: c_int, + actions: uw::_Unwind_Action, + _uw_exception_class: uw::_Unwind_Exception_Class, + _uw_exception: *mut uw::_Unwind_Exception, + context: *mut uw::_Unwind_Context, + _stop_parameter: *mut c_void) -> uw::_Unwind_Reason_Code { unsafe { - let exception_info = &mut *(uw_exception as *mut ExceptionInfo); - - if exception_info.backtrace_size < exception_info.backtrace.len() { + let backtrace_size = EXCEPTION_BUFFER.backtrace_size; + if backtrace_size < MAX_BACKTRACE_SIZE { let ip = uw::_Unwind_GetIP(context); - exception_info.backtrace[exception_info.backtrace_size] = ip; - exception_info.backtrace_size += 1; + let fp = uw::_Unwind_GetGR(context, UNW_FP_REG); + if PAYLOAD_ADDRESS == 0 || ip > PAYLOAD_ADDRESS { + let ip = ip - PAYLOAD_ADDRESS; + EXCEPTION_BUFFER.backtrace[backtrace_size] = (ip, fp); + EXCEPTION_BUFFER.backtrace_size += 1; + let last_index = EXCEPTION_BUFFER.exception_stack[EXCEPTION_BUFFER.exception_count - 1]; + assert!(last_index != -1); + let sp_info = &mut EXCEPTION_BUFFER.stack_pointers[last_index as usize]; + sp_info.stack_pointer = fp; + sp_info.current_backtrace_size = backtrace_size + 1; + } } - if actions as u32 & uw::_UA_END_OF_STACK as u32 != 0 { - ::terminate(&exception_info.exception.unwrap(), - exception_info.backtrace[..exception_info.backtrace_size].as_mut()) + uncaught_exception() } else { uw::_URC_NO_REASON } } } -// We can unfortunately not use mem::zeroed in a static, so Option<> is used as a workaround. -// See https://github.com/rust-lang/rust/issues/39498. -static mut INFLIGHT: ExceptionInfo = ExceptionInfo { - uw_exception: uw::_Unwind_Exception { - exception_class: EXCEPTION_CLASS, - exception_cleanup: cleanup, - private: [0; uw::unwinder_private_data_size], - }, - exception: None, - handled: true, - backtrace: [0; MAX_BACKTRACE_SIZE], - backtrace_size: 0 -}; +static EXCEPTION_ID_LOOKUP: [(&str, u32); 10] = [ + ("RuntimeError", 0), + ("RTIOUnderflow", 1), + ("RTIOOverflow", 2), + ("RTIODestinationUnreachable", 3), + ("DMAError", 4), + ("I2CError", 5), + ("CacheError", 6), + ("SPIError", 7), + ("ZeroDivisionError", 8), + ("IndexError", 9) +]; -#[export_name="__artiq_raise"] -#[unwind(allowed)] -pub unsafe extern fn raise(exception: *const Exception) -> ! { - // Zing! The Exception<'a> to Exception<'static> transmute is not really sound in case - // the exception is ever captured. Fortunately, they currently aren't, and we save - // on the hassle of having to allocate exceptions somewhere except on stack. - INFLIGHT.exception = Some(mem::transmute::>(*exception)); - INFLIGHT.handled = false; - - let result = uw::_Unwind_RaiseException(&mut INFLIGHT.uw_exception); - assert!(result == uw::_URC_END_OF_STACK); - - INFLIGHT.backtrace_size = 0; - let _result = _Unwind_ForcedUnwind(&mut INFLIGHT.uw_exception, - uncaught_exception, ptr::null_mut()); - unreachable!() -} - -#[export_name="__artiq_reraise"] -#[unwind(allowed)] -pub unsafe extern fn reraise() -> ! { - use cslice::AsCSlice; - - if INFLIGHT.handled { - match INFLIGHT.exception { - Some(ref exception) => raise(exception), - None => raise(&Exception { - name: "0:artiq.coredevice.exceptions.RuntimeError".as_c_slice(), - file: file!().as_c_slice(), - line: line!(), - column: column!(), - // https://github.com/rust-lang/rfcs/pull/1719 - function: "__artiq_reraise".as_c_slice(), - message: "No active exception to reraise".as_c_slice(), - param: [0, 0, 0] - }) +pub fn get_exception_id(name: &str) -> u32 { + for (n, id) in EXCEPTION_ID_LOOKUP.iter() { + if *n == name { + return *id } - } else { - uw::_Unwind_Resume(&mut INFLIGHT.uw_exception) } + unimplemented!("unallocated internal exception id") } + diff --git a/artiq/firmware/ksupport/lib.rs b/artiq/firmware/ksupport/lib.rs index d915d7e81..1a1528494 100644 --- a/artiq/firmware/ksupport/lib.rs +++ b/artiq/firmware/ksupport/lib.rs @@ -1,5 +1,5 @@ #![feature(lang_items, llvm_asm, panic_unwind, libc, unwind_attributes, - panic_info_message, nll)] + panic_info_message, nll, const_in_array_repeat_expressions)] #![no_std] extern crate libc; @@ -80,8 +80,9 @@ macro_rules! println { macro_rules! raise { ($name:expr, $message:expr, $param0:expr, $param1:expr, $param2:expr) => ({ use cslice::AsCSlice; + let name_id = $crate::eh_artiq::get_exception_id($name); let exn = $crate::eh_artiq::Exception { - name: concat!("0:artiq.coredevice.exceptions.", $name).as_c_slice(), + id: name_id, file: file!().as_c_slice(), line: line!(), column: column!(), @@ -164,12 +165,12 @@ extern fn rpc_recv(slot: *mut ()) -> usize { &Err(ref exception) => unsafe { eh_artiq::raise(&eh_artiq::Exception { - name: exception.name.as_bytes().as_c_slice(), - file: exception.file.as_bytes().as_c_slice(), + id: exception.id, + file: exception.file, line: exception.line, column: exception.column, - function: exception.function.as_bytes().as_c_slice(), - message: exception.message.as_bytes().as_c_slice(), + function: exception.function, + message: exception.message, param: exception.param }) } @@ -177,27 +178,13 @@ extern fn rpc_recv(slot: *mut ()) -> usize { }) } -fn terminate(exception: &eh_artiq::Exception, backtrace: &mut [usize]) -> ! { - let mut cursor = 0; - for index in 0..backtrace.len() { - if backtrace[index] > kernel_proto::KERNELCPU_PAYLOAD_ADDRESS { - backtrace[cursor] = backtrace[index] - kernel_proto::KERNELCPU_PAYLOAD_ADDRESS; - cursor += 1; - } - } - let backtrace = &mut backtrace.as_mut()[0..cursor]; - +fn terminate(exceptions: &'static [Option>], + stack_pointers: &'static [eh_artiq::StackPointerBacktrace], + backtrace: &mut [(usize, usize)]) -> ! { send(&RunException { - exception: kernel_proto::Exception { - name: str::from_utf8(exception.name.as_ref()).unwrap(), - file: str::from_utf8(exception.file.as_ref()).unwrap(), - line: exception.line, - column: exception.column, - function: str::from_utf8(exception.function.as_ref()).unwrap(), - message: str::from_utf8(exception.message.as_ref()).unwrap(), - param: exception.param, - }, - backtrace: backtrace + exceptions, + stack_pointers, + backtrace }); loop {} } @@ -472,6 +459,7 @@ unsafe fn attribute_writeback(typeinfo: *const ()) { #[no_mangle] pub unsafe fn main() { + eh_artiq::reset_exception_buffer(KERNELCPU_PAYLOAD_ADDRESS); let image = slice::from_raw_parts_mut(kernel_proto::KERNELCPU_PAYLOAD_ADDRESS as *mut u8, kernel_proto::KERNELCPU_LAST_ADDRESS - kernel_proto::KERNELCPU_PAYLOAD_ADDRESS); diff --git a/artiq/firmware/libeh/dwarf.rs b/artiq/firmware/libeh/dwarf.rs index f70290c46..d2e1f633e 100644 --- a/artiq/firmware/libeh/dwarf.rs +++ b/artiq/firmware/libeh/dwarf.rs @@ -202,8 +202,7 @@ unsafe fn get_ttype_entry( pub unsafe fn find_eh_action( lsda: *const u8, context: &EHContext<'_>, - name: *const u8, - len: usize, + id: u32, ) -> Result { if lsda.is_null() { return Ok(EHAction::None); @@ -275,19 +274,9 @@ pub unsafe fn find_eh_action( return Ok(EHAction::Catch(lpad)); } // this seems to be target dependent - let clause_ptr = *(catch_type as *const CSlice); - let clause_name_ptr = (clause_ptr).as_ptr(); - let clause_name_len = (clause_ptr).len(); - if clause_name_len == len { - if (clause_name_ptr == core::ptr::null() || - clause_name_ptr == name || - // somehow their name pointers might differ, but the content is the - // same - core::slice::from_raw_parts(clause_name_ptr, clause_name_len) == - core::slice::from_raw_parts(name, len)) - { - return Ok(EHAction::Catch(lpad)); - } + let clause_id = *(catch_type as *const u32); + if clause_id == id { + return Ok(EHAction::Catch(lpad)); } } else if ar_filter < 0 { // FIXME: how to handle this? diff --git a/artiq/firmware/libeh/eh_artiq.rs b/artiq/firmware/libeh/eh_artiq.rs new file mode 100644 index 000000000..72e9e8d02 --- /dev/null +++ b/artiq/firmware/libeh/eh_artiq.rs @@ -0,0 +1,47 @@ +// ARTIQ Exception struct declaration +use cslice::CSlice; + +// Note: CSlice within an exception may not be actual cslice, they may be strings that exist only +// in the host. If the length == usize:MAX, the pointer is actually a string key in the host. +#[repr(C)] +#[derive(Copy, Clone)] +pub struct Exception<'a> { + pub id: u32, + pub file: CSlice<'a, u8>, + pub line: u32, + pub column: u32, + pub function: CSlice<'a, u8>, + pub message: CSlice<'a, u8>, + pub param: [i64; 3] +} + +fn str_err(_: core::str::Utf8Error) -> core::fmt::Error { + core::fmt::Error +} + +fn exception_str<'a>(s: &'a CSlice<'a, u8>) -> Result<&'a str, core::str::Utf8Error> { + if s.len() == usize::MAX { + Ok("") + } else { + core::str::from_utf8(s.as_ref()) + } +} + +impl<'a> core::fmt::Debug for Exception<'a> { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "Exception {} from {} in {}:{}:{}, message: {}", + self.id, + exception_str(&self.function).map_err(str_err)?, + exception_str(&self.file).map_err(str_err)?, + self.line, self.column, + exception_str(&self.message).map_err(str_err)?) + } +} + +#[derive(Copy, Clone, Debug, Default)] +pub struct StackPointerBacktrace { + pub stack_pointer: usize, + pub initial_backtrace_size: usize, + pub current_backtrace_size: usize, +} + diff --git a/artiq/firmware/libeh/eh_rust.rs b/artiq/firmware/libeh/eh_rust.rs index 43e17eeac..886d00aa2 100644 --- a/artiq/firmware/libeh/eh_rust.rs +++ b/artiq/firmware/libeh/eh_rust.rs @@ -87,5 +87,5 @@ unsafe fn find_eh_action(context: *mut uw::_Unwind_Context) -> Result { - pub name: &'a str, - pub file: &'a str, - pub line: u32, - pub column: u32, - pub function: &'a str, - pub message: &'a str, - pub param: [i64; 3] -} - #[derive(Debug)] pub enum Message<'a> { LoadRequest(&'a [u8]), @@ -47,8 +36,9 @@ pub enum Message<'a> { RunFinished, RunException { - exception: Exception<'a>, - backtrace: &'a [usize] + exceptions: &'a [Option>], + stack_pointers: &'a [eh::eh_artiq::StackPointerBacktrace], + backtrace: &'a [(usize, usize)] }, RunAborted, @@ -59,7 +49,7 @@ pub enum Message<'a> { data: *const *const () }, RpcRecvRequest(*mut ()), - RpcRecvReply(Result>), + RpcRecvReply(Result>), RpcFlush, CacheGetRequest { key: &'a str }, diff --git a/artiq/firmware/libproto_artiq/lib.rs b/artiq/firmware/libproto_artiq/lib.rs index d343d7d61..cb344616b 100644 --- a/artiq/firmware/libproto_artiq/lib.rs +++ b/artiq/firmware/libproto_artiq/lib.rs @@ -13,6 +13,7 @@ extern crate log; extern crate byteorder; extern crate io; extern crate dyld; +extern crate eh; // Internal protocols. pub mod kernel_proto; diff --git a/artiq/firmware/libproto_artiq/session_proto.rs b/artiq/firmware/libproto_artiq/session_proto.rs index 0475a4489..523331416 100644 --- a/artiq/firmware/libproto_artiq/session_proto.rs +++ b/artiq/firmware/libproto_artiq/session_proto.rs @@ -1,5 +1,7 @@ use core::str::Utf8Error; -use alloc::{vec::Vec, string::String}; +use alloc::vec::Vec; +use eh::eh_artiq::{Exception, StackPointerBacktrace}; +use cslice::CSlice; use io::{Read, ProtoRead, Write, ProtoWrite, Error as IoError, ReadStringError}; @@ -70,13 +72,13 @@ pub enum Request { RpcReply { tag: Vec }, RpcException { - name: String, - message: String, + id: u32, + message: u32, param: [i64; 3], - file: String, + file: u32, line: u32, column: u32, - function: String, + function: u32, }, } @@ -95,14 +97,9 @@ pub enum Reply<'a> { }, KernelStartupFailed, KernelException { - name: &'a str, - message: &'a str, - param: [i64; 3], - file: &'a str, - line: u32, - column: u32, - function: &'a str, - backtrace: &'a [usize], + exceptions: &'a [Option>], + stack_pointers: &'a [StackPointerBacktrace], + backtrace: &'a [(usize, usize)], async_errors: u8 }, @@ -126,15 +123,15 @@ impl Request { tag: reader.read_bytes()? }, 8 => Request::RpcException { - name: reader.read_string()?, - message: reader.read_string()?, + id: reader.read_u32()?, + message: reader.read_u32()?, param: [reader.read_u64()? as i64, reader.read_u64()? as i64, reader.read_u64()? as i64], - file: reader.read_string()?, + file: reader.read_u32()?, line: reader.read_u32()?, column: reader.read_u32()?, - function: reader.read_string()? + function: reader.read_u32()? }, ty => return Err(Error::UnknownPacket(ty)) @@ -142,6 +139,18 @@ impl Request { } } +fn write_exception_string<'a, W>(writer: &mut W, s: &CSlice<'a, u8>) -> Result<(), IoError> + where W: Write + ?Sized +{ + if s.len() == usize::MAX { + writer.write_u32(u32::MAX)?; + writer.write_u32(s.as_ptr() as u32)?; + } else { + writer.write_string(core::str::from_utf8(s.as_ref()).unwrap())?; + } + Ok(()) +} + impl<'a> Reply<'a> { pub fn write_to(&self, writer: &mut W) -> Result<(), IoError> where W: Write + ?Sized @@ -171,22 +180,36 @@ impl<'a> Reply<'a> { writer.write_u8(8)?; }, Reply::KernelException { - name, message, param, file, line, column, function, backtrace, + exceptions, + stack_pointers, + backtrace, async_errors } => { writer.write_u8(9)?; - writer.write_string(name)?; - writer.write_string(message)?; - writer.write_u64(param[0] as u64)?; - writer.write_u64(param[1] as u64)?; - writer.write_u64(param[2] as u64)?; - writer.write_string(file)?; - writer.write_u32(line)?; - writer.write_u32(column)?; - writer.write_string(function)?; + writer.write_u32(exceptions.len() as u32)?; + for exception in exceptions.iter() { + let exception = exception.as_ref().unwrap(); + writer.write_u32(exception.id as u32)?; + write_exception_string(writer, &exception.message)?; + writer.write_u64(exception.param[0] as u64)?; + writer.write_u64(exception.param[1] as u64)?; + writer.write_u64(exception.param[2] as u64)?; + write_exception_string(writer, &exception.file)?; + writer.write_u32(exception.line)?; + writer.write_u32(exception.column)?; + write_exception_string(writer, &exception.function)?; + } + + for sp in stack_pointers.iter() { + writer.write_u32(sp.stack_pointer as u32)?; + writer.write_u32(sp.initial_backtrace_size as u32)?; + writer.write_u32(sp.current_backtrace_size as u32)?; + } + writer.write_u32(backtrace.len() as u32)?; - for &addr in backtrace { - writer.write_u32(addr as u32)? + for &(addr, sp) in backtrace { + writer.write_u32(addr as u32)?; + writer.write_u32(sp as u32)?; } writer.write_u8(async_errors)?; }, diff --git a/artiq/firmware/runtime/session.rs b/artiq/firmware/runtime/session.rs index 260a1b385..bf374eb5d 100644 --- a/artiq/firmware/runtime/session.rs +++ b/artiq/firmware/runtime/session.rs @@ -1,6 +1,7 @@ use core::{mem, str, cell::{Cell, RefCell}, fmt::Write as FmtWrite}; use alloc::{vec::Vec, string::String}; use byteorder::{ByteOrder, NativeEndian}; +use cslice::CSlice; use io::{Read, Write, Error as IoError}; use board_misoc::{ident, cache, config}; @@ -291,7 +292,7 @@ fn process_host_message(io: &Io, } host::Request::RpcException { - name, message, param, file, line, column, function + id, message, param, file, line, column, function } => { if session.kernel_state != KernelState::RpcWait { unexpected!("unsolicited RPC reply") @@ -305,16 +306,18 @@ fn process_host_message(io: &Io, } })?; - let exn = kern::Exception { - name: name.as_ref(), - message: message.as_ref(), - param: param, - file: file.as_ref(), - line: line, - column: column, - function: function.as_ref() - }; - kern_send(io, &kern::RpcRecvReply(Err(exn)))?; + unsafe { + let exn = eh::eh_artiq::Exception { + id: id, + message: CSlice::new(message as *const u8, usize::MAX), + param: param, + file: CSlice::new(file as *const u8, usize::MAX), + line: line, + column: column, + function: CSlice::new(function as *const u8, usize::MAX), + }; + kern_send(io, &kern::RpcRecvReply(Err(exn)))?; + } session.kernel_state = KernelState::Running } @@ -438,7 +441,8 @@ fn process_kern_message(io: &Io, aux_mutex: &Mutex, } } &kern::RunException { - exception: kern::Exception { name, message, param, file, line, column, function }, + exceptions, + stack_pointers, backtrace } => { unsafe { kernel::stop() } @@ -448,19 +452,15 @@ fn process_kern_message(io: &Io, aux_mutex: &Mutex, match stream { None => { error!("exception in flash kernel"); - error!("{}: {} {:?}", name, message, param); - error!("at {}:{}:{} in {}", file, line, column, function); + for exception in exceptions { + error!("{:?}", exception.unwrap()); + } return Ok(true) }, Some(ref mut stream) => { host_write(stream, host::Reply::KernelException { - name: name, - message: message, - param: param, - file: file, - line: line, - column: column, - function: function, + exceptions: exceptions, + stack_pointers: stack_pointers, backtrace: backtrace, async_errors: unsafe { get_async_errors() } }).map_err(|e| e.into()) diff --git a/artiq/test/libartiq_support/lib.rs b/artiq/test/libartiq_support/lib.rs index 050ca31f2..4216c85ec 100644 --- a/artiq/test/libartiq_support/lib.rs +++ b/artiq/test/libartiq_support/lib.rs @@ -1,4 +1,4 @@ -#![feature(libc, panic_unwind, unwind_attributes, rustc_private, int_bits_const)] +#![feature(libc, panic_unwind, unwind_attributes, rustc_private, int_bits_const, const_in_array_repeat_expressions)] #![crate_name = "artiq_support"] #![crate_type = "cdylib"] @@ -58,23 +58,31 @@ mod cslice { pub mod eh { #[path = "../../firmware/libeh/dwarf.rs"] pub mod dwarf; + #[path = "../../firmware/libeh/eh_artiq.rs"] + pub mod eh_artiq; } #[path = "../../firmware/ksupport/eh_artiq.rs"] pub mod eh_artiq; use std::{str, process}; -fn terminate(exception: &eh_artiq::Exception, mut _backtrace: &mut [usize]) -> ! { - println!("Uncaught {}: {} ({}, {}, {})", - str::from_utf8(exception.name.as_ref()).unwrap(), - str::from_utf8(exception.message.as_ref()).unwrap(), - exception.param[0], - exception.param[1], - exception.param[2]); - println!("at {}:{}:{}", - str::from_utf8(exception.file.as_ref()).unwrap(), - exception.line, - exception.column); +fn terminate(exceptions: &'static [Option>], + _stack_pointers: &'static [eh_artiq::StackPointerBacktrace], + _backtrace: &'static mut [(usize, usize)]) -> ! { + println!("{}", exceptions.len()); + for exception in exceptions.iter() { + let exception = exception.as_ref().unwrap(); + println!("Uncaught {}: {} ({}, {}, {})", + exception.id, + str::from_utf8(exception.message.as_ref()).unwrap(), + exception.param[0], + exception.param[1], + exception.param[2]); + println!("at {}:{}:{}", + str::from_utf8(exception.file.as_ref()).unwrap(), + exception.line, + exception.column); + } process::exit(1); } diff --git a/flake.nix b/flake.nix index 8918092b0..78c9dc823 100644 --- a/flake.nix +++ b/flake.nix @@ -257,7 +257,7 @@ cargoDeps = rustPlatform.fetchCargoTarball { name = "artiq-firmware-cargo-deps"; src = "${self}/artiq/firmware"; - sha256 = "sha256-Lf6M4M/jdRiO5MsWSoqtOSNfRIhbze+qvg4kaiiBWW4="; + sha256 = "sha256-YyycMsDzR+JRcMZJd6A/CRi2J9nKmaWY/KXUnAQaZ00="; }; nativeBuildInputs = [ (pkgs.python3.withPackages(ps: [ migen misoc artiq ])) From 9d437626953fa01583e8944edf298c2b19d97cab Mon Sep 17 00:00:00 2001 From: pca006132 Date: Tue, 25 Jan 2022 15:15:57 +0800 Subject: [PATCH 8/8] test: fixed lit tests Note that because we changed exception representation from using string names as exception identifier into using integer IDs, we need to initialize the embedding map in order to allocate the integer IDs. Also, we can no longer print the exception names and messages from the kernel, we will need the host to map exception IDs to names, and may need the host to map string IDs to actual strings (messages can be static strings in the firmware, or strings stored in the host only). We now check for exception IDs for lit tests, which are fixed because we preallocated all builtin exceptions. --- artiq/compiler/embedding.py | 21 ++++++++++++------- artiq/compiler/module.py | 4 ++-- .../compiler/transforms/artiq_ir_generator.py | 7 ++++--- artiq/test/lit/exceptions/catch_all.py | 4 ++-- artiq/test/lit/exceptions/catch_multi.py | 4 ++-- artiq/test/lit/exceptions/reraise.py | 2 +- artiq/test/lit/exceptions/reraise_update.py | 2 +- artiq/test/lit/exceptions/uncaught.py | 2 +- 8 files changed, 27 insertions(+), 19 deletions(-) diff --git a/artiq/compiler/embedding.py b/artiq/compiler/embedding.py index 2a6ce62c5..359584d6d 100644 --- a/artiq/compiler/embedding.py +++ b/artiq/compiler/embedding.py @@ -50,9 +50,22 @@ class EmbeddingMap: self.str_forward_map = {} self.str_reverse_map = {} + self.preallocate_runtime_exception_names(["RuntimeError", + "RTIOUnderflow", + "RTIOOverflow", + "RTIODestinationUnreachable", + "DMAError", + "I2CError", + "CacheError", + "SPIError", + "0:ZeroDivisionError", + "0:IndexError"]) + def preallocate_runtime_exception_names(self, names): for i, name in enumerate(names): - exn_id = self.store_str("0:artiq.coredevice.exceptions." + name) + if ":" not in name: + name = "0:artiq.coredevice.exceptions." + name + exn_id = self.store_str(name) assert exn_id == i def store_str(self, s): @@ -754,12 +767,6 @@ class Stitcher: self.functions = {} self.embedding_map = EmbeddingMap() - self.embedding_map.preallocate_runtime_exception_names(["runtimeerror", - "RTIOUnderflow", - "RTIOOverflow", - "RTIODestinationUnreachable", - "DMAError", - "I2CError"]) self.value_map = defaultdict(lambda: []) self.definitely_changed = False diff --git a/artiq/compiler/module.py b/artiq/compiler/module.py index 7f027ba2f..f3bc35cde 100644 --- a/artiq/compiler/module.py +++ b/artiq/compiler/module.py @@ -10,7 +10,7 @@ string and infers types for it using a trivial :module:`prelude`. import os from pythonparser import source, diagnostic, parse_buffer -from . import prelude, types, transforms, analyses, validators +from . import prelude, types, transforms, analyses, validators, embedding class Source: def __init__(self, source_buffer, engine=None): @@ -18,7 +18,7 @@ class Source: self.engine = diagnostic.Engine(all_errors_are_fatal=True) else: self.engine = engine - self.embedding_map = None + self.embedding_map = embedding.EmbeddingMap() self.name, _ = os.path.splitext(os.path.basename(source_buffer.name)) asttyped_rewriter = transforms.ASTTypedRewriter(engine=engine, diff --git a/artiq/compiler/transforms/artiq_ir_generator.py b/artiq/compiler/transforms/artiq_ir_generator.py index bd69ca79c..40f50718a 100644 --- a/artiq/compiler/transforms/artiq_ir_generator.py +++ b/artiq/compiler/transforms/artiq_ir_generator.py @@ -2814,14 +2814,15 @@ class ARTIQIRGenerator(algorithm.Visitor): format_string += ")" elif builtins.is_exception(value.type): + # message may not be an actual string... + # so we cannot really print it name = self.append(ir.GetAttr(value, "#__name__")) - message = self.append(ir.GetAttr(value, "#__message__")) param1 = self.append(ir.GetAttr(value, "#__param0__")) param2 = self.append(ir.GetAttr(value, "#__param1__")) param3 = self.append(ir.GetAttr(value, "#__param2__")) - format_string += "%.*s(%.*s, %lld, %lld, %lld)" - args += [name, message, param1, param2, param3] + format_string += "%ld(%lld, %lld, %lld)" + args += [name, param1, param2, param3] else: assert False diff --git a/artiq/test/lit/exceptions/catch_all.py b/artiq/test/lit/exceptions/catch_all.py index 1417f5f31..1b4c38b8e 100644 --- a/artiq/test/lit/exceptions/catch_all.py +++ b/artiq/test/lit/exceptions/catch_all.py @@ -8,7 +8,7 @@ def catch(f): except Exception as e: print(e) -# CHECK-L: ZeroDivisionError +# CHECK-L: 8(0, 0, 0) catch(lambda: 1/0) -# CHECK-L: IndexError +# CHECK-L: 9(10, 1, 0) catch(lambda: [1.0][10]) diff --git a/artiq/test/lit/exceptions/catch_multi.py b/artiq/test/lit/exceptions/catch_multi.py index 472086660..dbd8bcdec 100644 --- a/artiq/test/lit/exceptions/catch_multi.py +++ b/artiq/test/lit/exceptions/catch_multi.py @@ -10,7 +10,7 @@ def catch(f): except IndexError as ie: print(ie) -# CHECK-L: ZeroDivisionError +# CHECK-L: 8(0, 0, 0) catch(lambda: 1/0) -# CHECK-L: IndexError +# CHECK-L: 9(10, 1, 0) catch(lambda: [1.0][10]) diff --git a/artiq/test/lit/exceptions/reraise.py b/artiq/test/lit/exceptions/reraise.py index 911b22322..fd5eabd4a 100644 --- a/artiq/test/lit/exceptions/reraise.py +++ b/artiq/test/lit/exceptions/reraise.py @@ -3,7 +3,7 @@ # REQUIRES: exceptions def f(): - # CHECK-L: Uncaught 0:ZeroDivisionError + # CHECK-L: Uncaught 8 # CHECK-L: at input.py:${LINE:+1}: 1/0 diff --git a/artiq/test/lit/exceptions/reraise_update.py b/artiq/test/lit/exceptions/reraise_update.py index 11bd30639..aac8cdae1 100644 --- a/artiq/test/lit/exceptions/reraise_update.py +++ b/artiq/test/lit/exceptions/reraise_update.py @@ -9,7 +9,7 @@ def g(): try: f() except Exception as e: - # CHECK-L: Uncaught 0:ZeroDivisionError + # CHECK-L: Uncaught 8 # CHECK-L: at input.py:${LINE:+1}: raise e diff --git a/artiq/test/lit/exceptions/uncaught.py b/artiq/test/lit/exceptions/uncaught.py index 13b83ec22..1d44d4b2a 100644 --- a/artiq/test/lit/exceptions/uncaught.py +++ b/artiq/test/lit/exceptions/uncaught.py @@ -2,6 +2,6 @@ # RUN: OutputCheck %s --file-to-check=%t # REQUIRES: exceptions -# CHECK-L: Uncaught 0:ZeroDivisionError: cannot divide by zero (0, 0, 0) +# CHECK-L: Uncaught 8: cannot divide by zero (0, 0, 0) # CHECK-L: at input.py:${LINE:+1}: 1/0