| Message ID | 20210212153953.4582-5-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tools: Support to use abi-dumper on libraries | expand |
Andrew Cooper writes ("[PATCH 04/10] tools/libxl: Fix uninitialised variable in libxl__domain_get_device_model_uid()"):
> The logic is sufficiently complicated I can't figure out if the complain is
> legitimate or not. There is exactly one path wanting kill_by_uid set to true,
> so default it to false and drop the existing workaround for this problem at
> other optimisation levels.
The place where it's used is here:
if (!rc && user) {
state->dm_runas = user;
if (kill_by_uid)
state->dm_kill_uid = GCSPRINTF("%ld",...
This is gated by !rc. So for this to be used uninitialised, we'd have
to get here with rc==0 but uninitialised kill_by_uid.
The label `out` is preceded by a nonzero assignment to rc.
All the `goto out` are preceded by either (i) nonzero assignment to
rc, or (ii) assignment to kill_by_uid and setting rc=0.
So the compiler is wrong.
If only we had sum types.
In the absence of sum types I suggest the following restructuring:
Change all the `rc = ERROR...; goto out;` to `goto err` and make `goto
out` be the success path only.
Ian.
diff --git a/tools/libs/light/libxl_dm.c b/tools/libs/light/libxl_dm.c index 291dee9b3f..1ca21e4b81 100644 --- a/tools/libs/light/libxl_dm.c +++ b/tools/libs/light/libxl_dm.c @@ -128,7 +128,7 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc, int rc; char *user; uid_t intended_uid = -1; - bool kill_by_uid; + bool kill_by_uid = false; /* Only qemu-upstream can run as a different uid */ if (b_info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN) @@ -176,7 +176,6 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc, LOGD(DEBUG, guest_domid, "dm_restrict disabled, starting QEMU as root"); user = NULL; /* Should already be null, but just in case */ - kill_by_uid = false; /* Keep older versions of gcc happy */ rc = 0; goto out; } @@ -227,7 +226,6 @@ static int libxl__domain_get_device_model_uid(libxl__gc *gc, LOGD(WARN, guest_domid, "Could not find user %s, falling back to %s", LIBXL_QEMU_USER_RANGE_BASE, LIBXL_QEMU_USER_SHARED); intended_uid = user_base->pw_uid; - kill_by_uid = false; rc = 0; goto out; }
Various version of gcc, when compiling with -Og, complain: libxl_dm.c: In function 'libxl__domain_get_device_model_uid': libxl_dm.c:256:12: error: 'kill_by_uid' may be used uninitialized in this function [-Werror=maybe-uninitialized] 256 | if (kill_by_uid) | ^ The logic is sufficiently complicated I can't figure out if the complain is legitimate or not. There is exactly one path wanting kill_by_uid set to true, so default it to false and drop the existing workaround for this problem at other optimisation levels. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Ian Jackson <iwj@xenproject.org> CC: Wei Liu <wl@xen.org> CC: Anthony PERARD <anthony.perard@citrix.com> --- tools/libs/light/libxl_dm.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)