https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89893

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |INVALID

--- Comment #8 from Martin Liška <marxin at gcc dot gnu.org> ---
Ok, so I did a deeper analysis and it looks there's a violation of aliasing:

bool PerIsolatePlatformData::FlushForegroundTasksInternal() {
  bool did_work = false;

  while (std::unique_ptr<DelayedTask> delayed =
      foreground_delayed_tasks_.Pop()) {
    did_work = true;
    uint64_t delay_millis =
        static_cast<uint64_t>(delayed->timeout + 0.5) * 1000;
    delayed->timer.data = static_cast<void*>(delayed.get());
    uv_timer_init(loop_, &delayed->timer);
    // Timers may not guarantee queue ordering of events with the same delay if
    // the delay is non-zero. This should not be a problem in practice.
    uv_timer_start(&delayed->timer, RunForegroundTask, delay_millis, 0);
    uv_unref(reinterpret_cast<uv_handle_t*>(&delayed->timer)); <---- HERE

    scheduled_delayed_tasks_.emplace_back(delayed.release(),
                                          [](DelayedTask* delayed) {
      uv_close(reinterpret_cast<uv_handle_t*>(&delayed->timer),  <---- HERE
               [](uv_handle_t* handle) {
        delete static_cast<DelayedTask*>(handle->data);
      });
    });
  }
  // Move all foreground tasks into a separate queue and flush that queue.
  // This way tasks that are posted while flushing the queue will be run on the
  // next call of FlushForegroundTasksInternal.
  std::queue<std::unique_ptr<Task>> tasks = foreground_tasks_.PopAll();
  while (!tasks.empty()) {
    std::unique_ptr<Task> task = std::move(tasks.front());
    tasks.pop();
    did_work = true;
    RunForegroundTask(std::move(task));
  }
  return did_work;
}

Taking look at struct definition I see:

struct uv_handle_s {
  void* data; uv_loop_t* loop; uv_handle_type type; uv_close_cb close_cb; void*
handle_queue[2]; union { int fd; void* reserved[4]; } u; uv_handle_t*
next_closing; unsigned int flags;
};
typedef struct uv_handle_s uv_handle_t;

typedef struct uv_timer_s uv_timer_t;
struct uv_timer_s {
  void* data; uv_loop_t* loop; uv_handle_type type; uv_close_cb close_cb; void*
handle_queue[2]; union { int fd; void* reserved[4]; } u; uv_handle_t*
next_closing; unsigned int flags;
  uv_timer_cb timer_cb; void* heap_node[3]; uint64_t timeout; uint64_t repeat;
uint64_t start_id;
};

When I use -fno-strict-aliasing, or when I mark uv_unref, then it's fine.
Similarly with -O2, the inling of uv_unref does not happen.


diff --git a/deps/uv/include/uv.h b/deps/uv/include/uv.h
index 717c2e5..6bb3985 100644
--- a/deps/uv/include/uv.h
+++ b/deps/uv/include/uv.h
@@ -283,7 +283,7 @@ UV_EXTERN int uv_run(uv_loop_t*, uv_run_mode mode);
 UV_EXTERN void uv_stop(uv_loop_t*);

 UV_EXTERN void uv_ref(uv_handle_t*);
-UV_EXTERN void uv_unref(uv_handle_t*);
+UV_EXTERN void  __attribute__((noipa)) uv_unref(uv_handle_t*);
 UV_EXTERN int uv_has_ref(const uv_handle_t*);

 UV_EXTERN void uv_update_time(uv_loop_t*);

I'm going to create an upstream issue for it.

Reply via email to