The zx_clock_get syscall on Fuchsia is deprecated - ref
https://fuchsia.dev/fuchsia-src/reference/syscalls/clock_get
This changes to the recommended replacement; calling zx_clock_read on
the userspace UTC clock.
Details
- Reviewers
mcgrathr phosek - Commits
- rG5a3077f3a7b3: [sanitizer][fuchsia] Avoid deprecated syscall.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This change is addressing fxbug.dev/61726 on the Fuchsia side. Note that this is my first time attempting to make an LLVM change; I've verified this compiles by building the Fuchsia toolchain but thats all.
Yes please in a little bit. There isn't any urgency here so I'm happy to leave open for a day or two in case anyone else has comments. In particular I'd like John to take a quick look, I can follow up offline when thats happened.
This is fine. If we wanted to avoid wider use of _zx_utc_reference_get, I think it would also be fine for this one call to use the C11 timespec_get API rather than a pure Zircon API.
Generally, this LGTM. The check against ZX_HANDLE_INVALID is probably redundant. zx_clock_read will reject the call if the handle is invalid, returning an error (which would be caught on line 55) or terminating the calling process (depending on process's policy). I'm fine with leaving the check in place if you want, however.
I do have one quick question, however. Is the implication of how this function works that there must be a UTC clock available to any process which runs with the sanitizer? IOW - what happens if (someday) we end up not granting access to UTC, and someone attempts to run their code with the sanitizer on?
This is probably not a great place to have this discussion, be let's talk about it some offline if you have some time (pun).
I did see a couple of places where it looked like NanoTime could be used as a fallback (e.g. scudo_allocator.cpp:280 if the GetRandom fails). Getting rid of NanoTime does sound possible and beneficial longer term but are you OK if this CL just removes the _zx_clock_get reference for now?
Yes it is the implication that there must be a UTC clock available for processes that end up invoking this function. As Roland points out though there are very few places this is invoked, and at the moment our intent is to supply the UTC clock to all processes to match the global availability of the kernel UTC clock. Getting rid of the function entirely does seem like a good idea long term.
I think the discussion has finished up now, so yes if you could land this for me I'd really appreciate it.