Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toml Python package v0.10.0 is buggy; need v0.10.2 #990

Closed
dimakuv opened this issue Oct 19, 2022 · 2 comments · Fixed by #995
Closed

toml Python package v0.10.0 is buggy; need v0.10.2 #990

dimakuv opened this issue Oct 19, 2022 · 2 comments · Fixed by #995

Comments

@dimakuv
Copy link
Contributor

dimakuv commented Oct 19, 2022

Description of the problem

While testing #671, I encountered this bug.

#671 introduces a rather complex format for IOCTL data structures. Example from device_ioctl.manifest.template from #671:

sgx.ioctl_structs.gramine_test_dev_ioctl_write = [
    { size=8, type="out", name="buf_size" },     # buf_size
    { ptr=[ {size="buf_size", type="out"} ] },   # buf
    { size=8, type="inout" },                    # off
    { adjust=-4, size=12, type="in" },           # copied; adjust is just for testing
]

Gramine uses the toml Python package, including for manifest.template rendering:

The toml Python package on Ubuntu 20.04 is taken from apt install:

python3-toml \

On Ubuntu 18.04, it is taken from pip3 install:

'toml>=0.10' \

So on Ubuntu 18.04, we get v0.10.2. But on Ubuntu 20.04, we get v0.10.0.

Unfortunately, v0.10.0 had a bug that directly affects my TOML snippet above: "Parsing fails on inline array of tables containing a nested table". This bug was fixed in v0.10.2 though.

So on Ubuntu 20.04, if the toml Python package is installed as apt install python3-toml (from Ubuntu repos), then such TOML manifests fail.

Proposed solutions:

  1. Switch to another TOML parser in Python. One suggestion was tomli.
  2. Require to use toml Python package v0.10.2. For this:
    • If Gramine is built from source, then the package must be installed via pip3 install toml.
    • If Gramine is installed via our packages, then all packages must contain python3-toml package with v0.10.2 -- this can be taken from e.g. Ubuntu 22.04.

@dimakuv and @woju are fine with solution 2, at least in the short-term. But we should discuss it during the Gramine meeting.

Steps to reproduce

Try #671 on Ubuntu 20.04 with toml Python package installed via apt install python3-toml.

Expected results

device_ioctl.manifest.template from #671 is rendered correctly by toml Python package.

Actual results

This is what I observed in #671 in Ubuntu 20.04 Jenkins jobs:

16:11:59  gramine-manifest -Darch_libdir=/lib/x86_64-linux-gnu -Dcoreutils_libdir=/usr/lib/x86_64-linux-gnu/coreutils -Dentrypoint=device_ioctl -Dbinary_dir=/home/jenkins/workspace/graphene-20.04/usr/lib/x86_64-linux-gnu/gramine/tests/libos/regression -Dlibc=glibc device_ioctl.manifest.template device_ioctl.manifest
16:11:59  Traceback (most recent call last):
16:11:59    File "/usr/lib/python3/dist-packages/toml/decoder.py", line 455, in loads
16:11:59      ret = decoder.load_line(line, currentlevel, multikey,
16:11:59    File "/usr/lib/python3/dist-packages/toml/decoder.py", line 725, in load_line
16:11:59      value, vtype = self.load_value(pair[1], strictly_valid)
16:11:59    File "/usr/lib/python3/dist-packages/toml/decoder.py", line 802, in load_value
16:11:59      return (self.load_array(v), "array")
16:11:59    File "/usr/lib/python3/dist-packages/toml/decoder.py", line 938, in load_array
16:11:59      nval, ntype = self.load_value(a[i])
16:11:59    File "/usr/lib/python3/dist-packages/toml/decoder.py", line 805, in load_value
16:11:59      self.load_inline_object(v, inline_object)
16:11:59    File "/usr/lib/python3/dist-packages/toml/decoder.py", line 621, in load_inline_object
16:11:59      raise ValueError("Invalid inline table value encountered")
16:11:59  ValueError: Invalid inline table value encountered
16:11:59  
16:11:59  During handling of the above exception, another exception occurred:
16:11:59  
16:11:59  Traceback (most recent call last):
16:11:59    File "/home/jenkins/workspace/graphene-20.04/usr/bin/gramine-manifest", line 33, in <module>
16:11:59      main() # pylint: disable=no-value-for-parameter
16:11:59    File "/usr/lib/python3/dist-packages/click/core.py", line 764, in __call__
16:11:59      return self.main(*args, **kwargs)
16:11:59    File "/usr/lib/python3/dist-packages/click/core.py", line 717, in main
16:11:59      rv = self.invoke(ctx)
16:11:59    File "/usr/lib/python3/dist-packages/click/core.py", line 956, in invoke
16:11:59      return ctx.invoke(self.callback, **ctx.params)
16:11:59    File "/usr/lib/python3/dist-packages/click/core.py", line 555, in invoke
16:11:59      return callback(*args, **kwargs)
16:11:59    File "/home/jenkins/workspace/graphene-20.04/usr/bin/gramine-manifest", line 29, in main
16:11:59      manifest = Manifest.from_template(template, define)
16:11:59    File "/home/jenkins/workspace/graphene-20.04/usr/lib/python3.8/site-packages/graminelibos/manifest.py", line 142, in from_template
16:11:59      return cls(_env.from_string(template).render(**(variables or {})))
16:11:59    File "/home/jenkins/workspace/graphene-20.04/usr/lib/python3.8/site-packages/graminelibos/manifest.py", line 84, in __init__
16:11:59      manifest = toml.loads(manifest_str)
16:11:59    File "/usr/lib/python3/dist-packages/toml/decoder.py", line 458, in loads
16:11:59      raise TomlDecodeError(str(err), original, pos)
16:11:59  toml.decoder.TomlDecodeError: Invalid inline table value encountered (line 34 column 1 char 1547)

Gramine commit hash

d3b65ab (in #671)

@mkow
Copy link
Member

mkow commented Oct 19, 2022

Let's just move to tomli instead of adding another hacks. The current toml library seems to be of really bad quality (see gramineproject/gsc#101).

@dimakuv
Copy link
Contributor Author

dimakuv commented Oct 20, 2022

Ok, I will create a PR to move to tomli.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants