From 7a912c55fb45582f88abcb907f577b7bd336032d Mon Sep 17 00:00:00 2001 From: Wei Ren <6765243+weiren2@users.noreply.github.com> Date: Sun, 27 Jul 2025 11:19:07 -0700 Subject: [PATCH 1/3] Fix casting from double to uint64_t to avoid UB --- vvp/vpi_vthr_vector.cc | 16 ++++++++++++---- vvp/vthread.cc | 23 ++++++++++++++++++----- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/vvp/vpi_vthr_vector.cc b/vvp/vpi_vthr_vector.cc index 7a3d0bd38..91e7b3e1a 100644 --- a/vvp/vpi_vthr_vector.cc +++ b/vvp/vpi_vthr_vector.cc @@ -96,6 +96,14 @@ static double vlg_round(double rval) } } +static uint64_t vlg_round_to_u64(double rval) +{ + // Directly casting a negative double to an unsigned integer types is + // undefined behavior and behaves differently on different architectures. + // Cast to signed integer first to get the behavior we want. + return static_cast(static_cast(vlg_round(rval))); +} + static void vthr_real_get_value(vpiHandle ref, s_vpi_value*vp) { __vpiVThrWord*obj = dynamic_cast<__vpiVThrWord*>(ref); @@ -141,17 +149,17 @@ static void vthr_real_get_value(vpiHandle ref, s_vpi_value*vp) break; case vpiOctStrVal: - snprintf(rbuf, RBUF_USE_SIZE, "%" PRIo64, (uint64_t)vlg_round(val)); + snprintf(rbuf, RBUF_USE_SIZE, "%" PRIo64, vlg_round_to_u64(val)); vp->value.str = rbuf; break; case vpiHexStrVal: - snprintf(rbuf, RBUF_USE_SIZE, "%" PRIx64, (uint64_t)vlg_round(val)); + snprintf(rbuf, RBUF_USE_SIZE, "%" PRIx64, vlg_round_to_u64(val)); vp->value.str = rbuf; break; case vpiBinStrVal: { - uint64_t vali = (uint64_t)vlg_round(val); + uint64_t vali = vlg_round_to_u64(val); unsigned len = 0; while (vali > 0) { @@ -159,7 +167,7 @@ static void vthr_real_get_value(vpiHandle ref, s_vpi_value*vp) vali /= 2; } - vali = (uint64_t)vlg_round(val); + vali = vlg_round_to_u64(val); for (unsigned idx = 0 ; idx < len ; idx += 1) { rbuf[len-idx-1] = (vali & 1)? '1' : '0'; vali /= 2; diff --git a/vvp/vthread.cc b/vvp/vthread.cc index deee63fd0..505e5f11c 100644 --- a/vvp/vthread.cc +++ b/vvp/vthread.cc @@ -2374,17 +2374,30 @@ bool of_CVT_SR(vthread_t thr, vvp_code_t cp) return true; } +static double vlg_round(double rval) +{ + if (rval >= 0.0) { + return floor(rval + 0.5); + } else { + return ceil(rval - 0.5); + } +} + +static uint64_t vlg_round_to_u64(double rval) +{ + // Directly casting a negative double to an unsigned integer types is + // undefined behavior and behaves differently on different architectures. + // Cast to signed integer first to get the behavior we want. + return static_cast(static_cast(vlg_round(rval))); +} + /* * %cvt/ur */ bool of_CVT_UR(vthread_t thr, vvp_code_t cp) { double r = thr->pop_real(); - if (r >= 0.0) - thr->words[cp->bit_idx[0]].w_uint = (uint64_t)floor(r+0.5); - else - thr->words[cp->bit_idx[0]].w_uint = (uint64_t)ceil(r-0.5); - + thr->words[cp->bit_idx[0]].w_uint = vlg_round_to_u64(r); return true; } From aa6ae73740b5bc38474c2d0f2985f0c69abc58be Mon Sep 17 00:00:00 2001 From: Wei Ren <6765243+weiren2@users.noreply.github.com> Date: Tue, 29 Jul 2025 00:23:34 -0700 Subject: [PATCH 2/3] Use rounding functions from and put `vlg_round_to_u64` into a common utility header. --- vvp/vpi_callback.cc | 22 +++------------------- vvp/vpi_utils.h | 15 +++++++++++++++ vvp/vpi_vthr_vector.cc | 22 +++------------------- vvp/vthread.cc | 18 +----------------- 4 files changed, 22 insertions(+), 55 deletions(-) create mode 100644 vvp/vpi_utils.h diff --git a/vvp/vpi_callback.cc b/vvp/vpi_callback.cc index e0d2e2c43..4fc8bcc48 100644 --- a/vvp/vpi_callback.cc +++ b/vvp/vpi_callback.cc @@ -26,6 +26,7 @@ # include "vpi_user.h" # include "vpi_priv.h" +# include "vpi_utils.h" # include "vvp_net.h" # include "schedule.h" # include "event.h" @@ -949,23 +950,6 @@ void vvp_signal_value::get_signal_value(struct t_vpi_value*vp) } } -static double vlg_round(double rval) -{ - if (rval >= 0.0) { - return floor(rval + 0.5); - } else { - return ceil(rval - 0.5); - } -} - -static uint64_t vlg_round_to_u64(double rval) -{ - // Directly casting a negative double to an unsigned integer types is - // undefined behavior and behaves differently on different architectures. - // Cast to signed integer first to get the behavior we want. - return static_cast(static_cast(vlg_round(rval))); -} - static void real_signal_value(struct t_vpi_value*vp, double rval) { static const size_t RBUF_SIZE = 64 + 1; @@ -984,7 +968,7 @@ static void real_signal_value(struct t_vpi_value*vp, double rval) if (rval != rval || (rval && (rval == 0.5*rval))) { rval = 0.0; } else { - rval = vlg_round(rval); + rval = std::round(rval); } vp->value.integer = (PLI_INT32)rval; break; @@ -993,7 +977,7 @@ static void real_signal_value(struct t_vpi_value*vp, double rval) if (std::isnan(rval)) snprintf(rbuf, RBUF_SIZE, "%s", "nan"); else - snprintf(rbuf, RBUF_SIZE, "%0.0f", vlg_round(rval)); + snprintf(rbuf, RBUF_SIZE, "%0.0f", std::round(rval)); vp->value.str = rbuf; break; diff --git a/vvp/vpi_utils.h b/vvp/vpi_utils.h new file mode 100644 index 000000000..244aff3f9 --- /dev/null +++ b/vvp/vpi_utils.h @@ -0,0 +1,15 @@ +#ifndef IVL_vpi_utils_H +#define IVL_vpi_utils_H + +#include +#include + +static inline uint64_t vlg_round_to_u64(double rval) +{ + // Directly casting a negative double to an unsigned integer types is + // undefined behavior and behaves differently on different architectures. + // Cast to signed integer first to get the behavior we want. + return static_cast(static_cast(std::llround(rval))); +} + +#endif diff --git a/vvp/vpi_vthr_vector.cc b/vvp/vpi_vthr_vector.cc index 91e7b3e1a..87ea1bc34 100644 --- a/vvp/vpi_vthr_vector.cc +++ b/vvp/vpi_vthr_vector.cc @@ -24,6 +24,7 @@ */ # include "vpi_priv.h" +# include "vpi_utils.h" # include "vthread.h" # include "config.h" #ifdef CHECK_WITH_VALGRIND @@ -87,23 +88,6 @@ static int vthr_word_get(int code, vpiHandle ref) } } -static double vlg_round(double rval) -{ - if (rval >= 0.0) { - return floor(rval + 0.5); - } else { - return ceil(rval - 0.5); - } -} - -static uint64_t vlg_round_to_u64(double rval) -{ - // Directly casting a negative double to an unsigned integer types is - // undefined behavior and behaves differently on different architectures. - // Cast to signed integer first to get the behavior we want. - return static_cast(static_cast(vlg_round(rval))); -} - static void vthr_real_get_value(vpiHandle ref, s_vpi_value*vp) { __vpiVThrWord*obj = dynamic_cast<__vpiVThrWord*>(ref); @@ -135,7 +119,7 @@ static void vthr_real_get_value(vpiHandle ref, s_vpi_value*vp) if (val != val || (val && (val == 0.5*val))) { val = 0.0; } else { - val = vlg_round(val); + val = std::round(val); } vp->value.integer = (PLI_INT32)val; break; @@ -144,7 +128,7 @@ static void vthr_real_get_value(vpiHandle ref, s_vpi_value*vp) if (std::isnan(val)) snprintf(rbuf, RBUF_USE_SIZE, "%s", "nan"); else - snprintf(rbuf, RBUF_USE_SIZE, "%0.0f", vlg_round(val)); + snprintf(rbuf, RBUF_USE_SIZE, "%0.0f", std::round(val)); vp->value.str = rbuf; break; diff --git a/vvp/vthread.cc b/vvp/vthread.cc index 505e5f11c..ba77178ee 100644 --- a/vvp/vthread.cc +++ b/vvp/vthread.cc @@ -24,6 +24,7 @@ # include "ufunc.h" # include "event.h" # include "vpi_priv.h" +# include "vpi_utils.h" # include "vvp_net_sig.h" # include "vvp_cobject.h" # include "vvp_darray.h" @@ -2374,23 +2375,6 @@ bool of_CVT_SR(vthread_t thr, vvp_code_t cp) return true; } -static double vlg_round(double rval) -{ - if (rval >= 0.0) { - return floor(rval + 0.5); - } else { - return ceil(rval - 0.5); - } -} - -static uint64_t vlg_round_to_u64(double rval) -{ - // Directly casting a negative double to an unsigned integer types is - // undefined behavior and behaves differently on different architectures. - // Cast to signed integer first to get the behavior we want. - return static_cast(static_cast(vlg_round(rval))); -} - /* * %cvt/ur */ From b35b74ac9ce889a012310be8f410084ebb4b64d3 Mon Sep 17 00:00:00 2001 From: Wei Ren <6765243+weiren2@users.noreply.github.com> Date: Tue, 29 Jul 2025 00:49:01 -0700 Subject: [PATCH 3/3] Refactor the logic for binary string value conversion. --- vvp/vpi_callback.cc | 36 +++++++++++++++++------------------- vvp/vpi_vthr_vector.cc | 35 ++++++++++++++++------------------- 2 files changed, 33 insertions(+), 38 deletions(-) diff --git a/vvp/vpi_callback.cc b/vvp/vpi_callback.cc index 4fc8bcc48..d09a89c18 100644 --- a/vvp/vpi_callback.cc +++ b/vvp/vpi_callback.cc @@ -987,28 +987,26 @@ static void real_signal_value(struct t_vpi_value*vp, double rval) break; case vpiBinStrVal: { - uint64_t val = vlg_round_to_u64(rval); - unsigned len = 0; + const uint64_t val = vlg_round_to_u64(rval); + unsigned len = 0; - while (val > 0) { - len += 1; - val /= 2; - } + // Compute bit‑width; For val==0, this yields len==1 + uint64_t tmp = val; + do { + len += 1; + tmp /= 2; + } while (tmp > 0); - val = vlg_round_to_u64(rval); - for (unsigned idx = 0 ; idx < len ; idx += 1) { - rbuf[len-idx-1] = (val & 1)? '1' : '0'; - val /= 2; - } + tmp = val; + for (unsigned idx = 0; idx < len; idx += 1) { + rbuf[len - idx - 1] = (tmp & 1) ? '1' : '0'; + tmp /= 2; + } - rbuf[len] = 0; - if (len == 0) { - rbuf[0] = '0'; - rbuf[1] = 0; - } - vp->value.str = rbuf; - break; - } + rbuf[len] = '\0'; + vp->value.str = rbuf; + break; + } case vpiSuppressVal: break; diff --git a/vvp/vpi_vthr_vector.cc b/vvp/vpi_vthr_vector.cc index 87ea1bc34..09c278f7f 100644 --- a/vvp/vpi_vthr_vector.cc +++ b/vvp/vpi_vthr_vector.cc @@ -143,28 +143,25 @@ static void vthr_real_get_value(vpiHandle ref, s_vpi_value*vp) break; case vpiBinStrVal: { - uint64_t vali = vlg_round_to_u64(val); - unsigned len = 0; + const uint64_t vali = vlg_round_to_u64(val); + unsigned len = 0; - while (vali > 0) { - len += 1; - vali /= 2; - } + uint64_t tmp = vali; + do { + len += 1; + tmp /= 2; + } while (tmp > 0); - vali = vlg_round_to_u64(val); - for (unsigned idx = 0 ; idx < len ; idx += 1) { - rbuf[len-idx-1] = (vali & 1)? '1' : '0'; - vali /= 2; - } + tmp = vali; + for (unsigned idx = 0; idx < len; idx += 1) { + rbuf[len - idx - 1] = (tmp & 1) ? '1' : '0'; + tmp /= 2; + } - rbuf[len] = 0; - if (len == 0) { - rbuf[0] = '0'; - rbuf[1] = 0; - } - vp->value.str = rbuf; - break; - } + rbuf[len] = '\0'; + vp->value.str = rbuf; + break; + } default: fprintf(stderr, "vvp error: get %d not supported "